From a33db254badce91b886a5973e743d3a251d2d760 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 5 Feb 2018 23:08:53 -0500 Subject: [PATCH] SOLR-11932 Retry ZkOperation on SessionExpired --- .../java/org/apache/solr/cloud/Overseer.java | 4 ++-- .../apache/solr/cloud/ZkSolrClientTest.java | 8 ++++---- .../solr/common/cloud/SolrZkClient.java | 20 +++++++++---------- .../solr/common/cloud/ZkCmdExecutor.java | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java index edf383884ea5..c72a03a01f8c 100644 --- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java +++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java @@ -303,7 +303,7 @@ private void checkIfIamStillLeader() { String path = OVERSEER_ELECT + "/leader"; byte[] data; try { - data = zkClient.getData(path, null, stat, true); + data = zkClient.getData(path, null, stat, false); } catch (Exception e) { log.error("could not read the data" ,e); return; @@ -314,7 +314,7 @@ private void checkIfIamStillLeader() { if(overseerCollectionConfigSetProcessor.getId().equals(id)){ try { log.warn("I'm exiting, but I'm still the leader"); - zkClient.delete(path,stat.getVersion(),true); + zkClient.delete(path,stat.getVersion(),false); } catch (KeeperException.BadVersionException e) { //no problem ignore it some other Overseer has already taken over } catch (Exception e) { diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkSolrClientTest.java b/solr/core/src/test/org/apache/solr/cloud/ZkSolrClientTest.java index fc40395b7f08..b4b25db09d45 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ZkSolrClientTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ZkSolrClientTest.java @@ -188,7 +188,7 @@ public void testReconnect() throws Exception { } } - public void testZkCmdExectutor() throws Exception { + public void testZkCmdExecutor() throws Exception { String zkDir = createTempDir("zkData").toFile().getAbsolutePath(); ZkTestServer server = null; @@ -205,14 +205,14 @@ public void testZkCmdExectutor() throws Exception { try { zkCmdExecutor.retryOperation(() -> { if (System.nanoTime() - start > TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS)) { - throw new KeeperException.SessionExpiredException(); + throw new KeeperException.AuthFailedException(); } throw new KeeperException.ConnectionLossException(); }); - } catch(KeeperException.SessionExpiredException e) { + } catch(KeeperException.AuthFailedException e) { } catch (Exception e) { - fail("Expected " + KeeperException.SessionExpiredException.class.getSimpleName() + " but got " + e.getClass().getSimpleName()); + fail("Expected " + KeeperException.AuthFailedException.class.getSimpleName() + " but got " + e.getClass().getSimpleName()); } } finally { if (server != null) { diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java index bc12c4469a5a..b873b5a5ed78 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java @@ -241,7 +241,7 @@ public void delete(final String path, final int version, boolean retryOnConnLoss throws InterruptedException, KeeperException { if (retryOnConnLoss) { zkCmdExecutor.retryOperation(() -> { - keeper.delete(path, version); + getSolrZooKeeper().delete(path, version); return null; }); } else { @@ -301,7 +301,7 @@ private interface SolrZkWatcher extends Watcher { public Stat exists(final String path, final Watcher watcher, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.exists(path, wrapWatcher(watcher))); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().exists(path, wrapWatcher(watcher))); } else { return keeper.exists(path, wrapWatcher(watcher)); } @@ -313,7 +313,7 @@ public Stat exists(final String path, final Watcher watcher, boolean retryOnConn public Boolean exists(final String path, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.exists(path, null) != null); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().exists(path, null) != null); } else { return keeper.exists(path, null) != null; } @@ -325,7 +325,7 @@ public Boolean exists(final String path, boolean retryOnConnLoss) public List getChildren(final String path, final Watcher watcher, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.getChildren(path, wrapWatcher(watcher))); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().getChildren(path, wrapWatcher(watcher))); } else { return keeper.getChildren(path, wrapWatcher(watcher)); } @@ -337,7 +337,7 @@ public List getChildren(final String path, final Watcher watcher, boolea public byte[] getData(final String path, final Watcher watcher, final Stat stat, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.getData(path, wrapWatcher(watcher), stat)); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().getData(path, wrapWatcher(watcher), stat)); } else { return keeper.getData(path, wrapWatcher(watcher), stat); } @@ -349,7 +349,7 @@ public byte[] getData(final String path, final Watcher watcher, final Stat stat, public Stat setData(final String path, final byte data[], final int version, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.setData(path, data, version)); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().setData(path, data, version)); } else { return keeper.setData(path, data, version); } @@ -362,7 +362,7 @@ public String create(final String path, final byte[] data, final CreateMode createMode, boolean retryOnConnLoss) throws KeeperException, InterruptedException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.create(path, data, zkACLProvider.getACLsToAdd(path), + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().create(path, data, zkACLProvider.getACLsToAdd(path), createMode)); } else { List acls = zkACLProvider.getACLsToAdd(path); @@ -493,7 +493,7 @@ public void makePath(String path, byte[] data, CreateMode createMode, final CreateMode finalMode = mode; final byte[] finalBytes = bytes; zkCmdExecutor.retryOperation(() -> { - keeper.create(currentPath, finalBytes, zkACLProvider.getACLsToAdd(currentPath), finalMode); + getSolrZooKeeper().create(currentPath, finalBytes, zkACLProvider.getACLsToAdd(currentPath), finalMode); return null; }); } else { @@ -555,7 +555,7 @@ public Stat setData(String path, File file, boolean retryOnConnLoss) throws IOEx public List multi(final Iterable ops, boolean retryOnConnLoss) throws InterruptedException, KeeperException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.multi(ops)); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().multi(ops)); } else { return keeper.multi(ops); } @@ -735,7 +735,7 @@ public ZkACLProvider getZkACLProvider() { */ public Stat setACL(String path, List acls, boolean retryOnConnLoss) throws InterruptedException, KeeperException { if (retryOnConnLoss) { - return zkCmdExecutor.retryOperation(() -> keeper.setACL(path, acls, -1)); + return zkCmdExecutor.retryOperation(() -> getSolrZooKeeper().setACL(path, acls, -1)); } else { return keeper.setACL(path, acls, -1); } diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java index c27f7671bc86..37698ebc2d9f 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java @@ -58,7 +58,7 @@ public T retryOperation(ZkOperation operation) for (int i = 0; i < retryCount; i++) { try { return (T) operation.execute(); - } catch (KeeperException.ConnectionLossException e) { + } catch (KeeperException.ConnectionLossException | KeeperException.SessionExpiredException e) { if (exception == null) { exception = e; }