That's annoying, since there's no semantic difference between that and a log.warn(fmt, arg, arg). Except that this particular logging API doesn't have an overload that takes a throwable, a format string, and a set of args.
On Fri, Feb 26, 2016 at 8:02 PM, Chris Hostetter <[email protected]> wrote: > > this breaks precommit... > > > [forbidden-apis] Forbidden method invocation: > java.lang.String#format(java.lang.String,java.lang.Object[]) [Uses default > locale] > [forbidden-apis] in org.apache.solr.cloud.OverseerTest$MockZKController > (OverseerTest.java:128) > [forbidden-apis] Scanned 3181 (and 1946 related) class file(s) for > forbidden API invocations (in 1.27s), 1 error(s). > > > > > : Date: Fri, 26 Feb 2016 17:32:25 +0000 (UTC) > : From: [email protected] > : Reply-To: [email protected] > : To: [email protected] > : Subject: lucene-solr git commit: SOLR-8697: Add synchronization around > : registering as leader and canceling. > : > : Repository: lucene-solr > : Updated Branches: > : refs/heads/master 0ed625b10 -> efb7bb171 > : > : > : SOLR-8697: Add synchronization around registering as leader and > canceling. > : > : > : Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo > : Commit: > http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/efb7bb17 > : Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/efb7bb17 > : Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/efb7bb17 > : > : Branch: refs/heads/master > : Commit: efb7bb171b22a3c6a00d30eefe935a0024df0c71 > : Parents: 0ed625b > : Author: markrmiller <[email protected]> > : Authored: Fri Feb 26 12:32:12 2016 -0500 > : Committer: markrmiller <[email protected]> > : Committed: Fri Feb 26 12:32:12 2016 -0500 > : > : ---------------------------------------------------------------------- > : .../org/apache/solr/cloud/ElectionContext.java | 110 > ++++++++++--------- > : .../org/apache/solr/cloud/ZkController.java | 2 +- > : .../org/apache/solr/cloud/OverseerTest.java | 7 ++ > : 3 files changed, 69 insertions(+), 50 deletions(-) > : ---------------------------------------------------------------------- > : > : > : > http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/efb7bb17/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > : ---------------------------------------------------------------------- > : diff --git > a/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > b/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > : index da4b0c6..6743436 100644 > : --- a/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > : +++ b/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > : @@ -110,8 +110,11 @@ class ShardLeaderElectionContextBase extends > ElectionContext { > : protected String shardId; > : protected String collection; > : protected LeaderElector leaderElector; > : - protected volatile Integer leaderZkNodeParentVersion; > : - > : + private Integer leaderZkNodeParentVersion; > : + > : + // Prevents a race between cancelling and becoming leader. > : + private final Object lock = new Object(); > : + > : public ShardLeaderElectionContextBase(LeaderElector leaderElector, > : final String shardId, final String collection, final String > coreNodeName, > : ZkNodeProps props, ZkStateReader zkStateReader) { > : @@ -138,31 +141,33 @@ class ShardLeaderElectionContextBase extends > ElectionContext { > : @Override > : public void cancelElection() throws InterruptedException, > KeeperException { > : super.cancelElection(); > : - if (leaderZkNodeParentVersion != null) { > : - try { > : - // We need to be careful and make sure we *only* delete our own > leader registration node. > : - // We do this by using a multi and ensuring the parent znode of > the leader registration node > : - // matches the version we expect - there is a setData call that > increments the parent's znode > : - // version whenever a leader registers. > : - log.info("Removing leader registration node on cancel: {} {}", > leaderPath, leaderZkNodeParentVersion); > : - List<Op> ops = new ArrayList<>(2); > : - ops.add(Op.check(new Path(leaderPath).getParent().toString(), > leaderZkNodeParentVersion)); > : - ops.add(Op.delete(leaderPath, -1)); > : - zkClient.multi(ops, true); > : - } catch (KeeperException.NoNodeException nne) { > : - // no problem > : - log.info("No leader registration node found to remove: {}", > leaderPath); > : - } catch (KeeperException.BadVersionException bve) { > : - log.info("Cannot remove leader registration node because the > current registered node is not ours: {}", leaderPath); > : - // no problem > : - } catch (InterruptedException e) { > : - throw e; > : - } catch (Exception e) { > : - SolrException.log(log, e); > : + synchronized (lock) { > : + if (leaderZkNodeParentVersion != null) { > : + try { > : + // We need to be careful and make sure we *only* delete our > own leader registration node. > : + // We do this by using a multi and ensuring the parent znode > of the leader registration node > : + // matches the version we expect - there is a setData call > that increments the parent's znode > : + // version whenever a leader registers. > : + log.info("Removing leader registration node on cancel: {} > {}", leaderPath, leaderZkNodeParentVersion); > : + List<Op> ops = new ArrayList<>(2); > : + ops.add(Op.check(new Path(leaderPath).getParent().toString(), > leaderZkNodeParentVersion)); > : + ops.add(Op.delete(leaderPath, -1)); > : + zkClient.multi(ops, true); > : + } catch (KeeperException.NoNodeException nne) { > : + // no problem > : + log.info("No leader registration node found to remove: {}", > leaderPath); > : + } catch (KeeperException.BadVersionException bve) { > : + log.info("Cannot remove leader registration node because the > current registered node is not ours: {}", leaderPath); > : + // no problem > : + } catch (InterruptedException e) { > : + throw e; > : + } catch (Exception e) { > : + SolrException.log(log, e); > : + } > : + leaderZkNodeParentVersion = null; > : + } else { > : + log.info("No version found for ephemeral leader parent node, > won't remove previous leader registration."); > : } > : - leaderZkNodeParentVersion = null; > : - } else { > : - log.info("No version found for ephemeral leader parent node, > won't remove previous leader registration."); > : } > : } > : > : @@ -179,30 +184,31 @@ class ShardLeaderElectionContextBase extends > ElectionContext { > : > : @Override > : public void execute() throws InterruptedException, > KeeperException { > : - log.info("Creating leader registration node {} after winning > as {}", leaderPath, leaderSeqPath); > : - List<Op> ops = new ArrayList<>(2); > : - > : - // We use a multi operation to get the parent nodes version, > which will > : - // be used to make sure we only remove our own leader > registration node. > : - // The setData call used to get the parent version is also > the trigger to > : - // increment the version. We also do a sanity check that our > leaderSeqPath exists. > : - > : - ops.add(Op.check(leaderSeqPath, -1)); > : - ops.add(Op.create(leaderPath, Utils.toJSON(leaderProps), > zkClient.getZkACLProvider().getACLsToAdd(leaderPath), > CreateMode.EPHEMERAL)); > : - ops.add(Op.setData(parent, null, -1)); > : - List<OpResult> results; > : - > : - results = zkClient.multi(ops, true); > : - > : - for (OpResult result : results) { > : - if (result.getType() == ZooDefs.OpCode.setData) { > : - SetDataResult dresult = (SetDataResult) result; > : - Stat stat = dresult.getStat(); > : - leaderZkNodeParentVersion = stat.getVersion(); > : - return; > : + synchronized (lock) { > : + log.info("Creating leader registration node {} after > winning as {}", leaderPath, leaderSeqPath); > : + List<Op> ops = new ArrayList<>(2); > : + > : + // We use a multi operation to get the parent nodes > version, which will > : + // be used to make sure we only remove our own leader > registration node. > : + // The setData call used to get the parent version is also > the trigger to > : + // increment the version. We also do a sanity check that > our leaderSeqPath exists. > : + > : + ops.add(Op.check(leaderSeqPath, -1)); > : + ops.add(Op.create(leaderPath, Utils.toJSON(leaderProps), > zkClient.getZkACLProvider().getACLsToAdd(leaderPath), > CreateMode.EPHEMERAL)); > : + ops.add(Op.setData(parent, null, -1)); > : + List<OpResult> results; > : + > : + results = zkClient.multi(ops, true); > : + for (OpResult result : results) { > : + if (result.getType() == ZooDefs.OpCode.setData) { > : + SetDataResult dresult = (SetDataResult) result; > : + Stat stat = dresult.getStat(); > : + leaderZkNodeParentVersion = stat.getVersion(); > : + return; > : + } > : } > : + assert leaderZkNodeParentVersion != null; > : } > : - assert leaderZkNodeParentVersion != null; > : } > : }); > : } catch (Throwable t) { > : @@ -225,7 +231,13 @@ class ShardLeaderElectionContextBase extends > ElectionContext { > : > : public LeaderElector getLeaderElector() { > : return leaderElector; > : - } > : + } > : + > : + Integer getLeaderZkNodeParentVersion() { > : + synchronized (lock) { > : + return leaderZkNodeParentVersion; > : + } > : + } > : } > : > : // add core container and stop passing core around... > : > : > http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/efb7bb17/solr/core/src/java/org/apache/solr/cloud/ZkController.java > : ---------------------------------------------------------------------- > : diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java > b/solr/core/src/java/org/apache/solr/cloud/ZkController.java > : index 4c826a7..aba2e59 100644 > : --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java > : +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java > : @@ -2120,7 +2120,7 @@ public final class ZkController { > : // we use this version and multi to ensure *only* the current zk > registered leader > : // for a shard can put a replica into LIR > : > : - Integer leaderZkNodeParentVersion = > ((ShardLeaderElectionContextBase)context).leaderZkNodeParentVersion; > : + Integer leaderZkNodeParentVersion = > ((ShardLeaderElectionContextBase)context).getLeaderZkNodeParentVersion(); > : > : // TODO: should we do this optimistically to avoid races? > : if (zkClient.exists(znodePath, retryOnConnLoss)) { > : > : > http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/efb7bb17/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > : ---------------------------------------------------------------------- > : diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > : index 8ac0512..ea82cbf 100644 > : --- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > : +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > : @@ -121,6 +121,13 @@ public class OverseerTest extends SolrTestCaseJ4 { > : } > : > : public void close() { > : + for (ElectionContext ec : electionContext.values()) { > : + try { > : + ec.cancelElection(); > : + } catch (Exception e) { > : + log.warn(String.format("Error cancelling election for %s", > ec.id), e); > : + } > : + } > : deleteNode(ZkStateReader.LIVE_NODES_ZKNODE + "/" + nodeName); > : zkClient.close(); > : } > : > : > > -Hoss > http://www.lucidworks.com/ > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
