From 7c28d5797387fdf68379ec66f0de3c62e6a079ca Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 21 Jun 2018 14:00:12 -0400 Subject: [PATCH] SOLR-12413: Ensure pre-loaded aliases in ZK are considered. (fix use of zk version in Aliases.EMPTY) --- solr/CHANGES.txt | 3 +++ .../org/apache/solr/common/cloud/Aliases.java | 6 +++--- .../apache/solr/common/cloud/ZkStateReader.java | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 118267297d7..960baf238d8 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -102,6 +102,9 @@ Bug Fixes * SOLR-11807: Restoring collection now treats maxShardsPerNode=-1 as unlimited (Varun Thacker) +* SOLR-12413: If Zookeeper was pre-loaded with data before first-use, then the aliases information would be ignored. + (David Smiley, Gaƫl Jourdan, Gus Heck) + Optimizations ---------------------- diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java index 671c27b0534..439b9363eeb 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java @@ -39,10 +39,10 @@ public class Aliases { /** * An empty, minimal Aliases primarily used to support the non-cloud solr use cases. Not normally useful * in cloud situations where the version of the node needs to be tracked even if all aliases are removed. - * A version of 0 is provided rather than -1 to minimize the possibility that if this is used in a cloud - * instance data is written without version checking. + * The -1 version makes it subordinate to any real version, and furthermore we never "set" this EMPTY instance + * into ZK. */ - public static final Aliases EMPTY = new Aliases(Collections.emptyMap(), Collections.emptyMap(), 0); + public static final Aliases EMPTY = new Aliases(Collections.emptyMap(), Collections.emptyMap(), -1); // These two constants correspond to the top level elements in aliases.json. The first one denotes // a section containing a list of aliases and their attendant collections, the second contains a list of diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index a86c5e28448..2c1209e2ff6 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -1698,6 +1698,19 @@ public class ZkStateReader implements Closeable { * The caller should understand it's possible the aliases has further changed if it examines it. */ public void applyModificationAndExportToZk(UnaryOperator op) { + // The current aliases hasn't been update()'ed yet -- which is impossible? Any way just update it first. + if (aliases.getZNodeVersion() == -1) { + try { + boolean updated = update(); + assert updated; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new ZooKeeperException(ErrorCode.SERVER_ERROR, e.toString(), e); + } catch (KeeperException e) { + throw new ZooKeeperException(ErrorCode.SERVER_ERROR, e.toString(), e); + } + } + final long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(30); // note: triesLeft tuning is based on ConcurrentCreateRoutedAliasTest for (int triesLeft = 30; triesLeft > 0; triesLeft--) { @@ -1769,6 +1782,7 @@ public class ZkStateReader implements Closeable { // note: it'd be nice to avoid possibly needlessly parsing if we don't update aliases but not a big deal setIfNewer(Aliases.fromJSON(data, stat.getVersion())); } catch (KeeperException.ConnectionLossException | KeeperException.SessionExpiredException e) { + // note: aliases.json is required to be present LOG.warn("ZooKeeper watch triggered, but Solr cannot talk to ZK: [{}]", e.getMessage()); } catch (KeeperException e) { LOG.error("A ZK error has occurred", e); @@ -1786,6 +1800,7 @@ public class ZkStateReader implements Closeable { * @param newAliases the potentially newer version of Aliases */ private boolean setIfNewer(Aliases newAliases) { + assert newAliases.getZNodeVersion() >= 0; synchronized (this) { int cmp = Integer.compare(aliases.getZNodeVersion(), newAliases.getZNodeVersion()); if (cmp < 0) {