This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.5 by this push:
new f20f0bc ZOOKEEPER-3223: Configure spotbugs - part 2
f20f0bc is described below
commit f20f0bcb1a35c4cfbf6d80b431a2abe67e954fb5
Author: Enrico Olivelli <[email protected]>
AuthorDate: Wed Jan 23 16:16:19 2019 +0100
ZOOKEEPER-3223: Configure spotbugs - part 2
- move to spotbugs 3.1.9
- disable spotbugs on contrib package
- fix spotbugs warnings on recipes
- add commons-lang 2.6 dependency in order to fix build
Author: Enrico Olivelli <[email protected]>
Reviewers: [email protected]
Closes #783 from eolivelli/fix/spotbugs-branch-3.5-part2
---
build.xml | 2 +-
pom.xml | 10 ++-
zookeeper-contrib/pom.xml | 18 +++++-
zookeeper-recipes/pom.xml | 2 +-
.../recipes/leader/LeaderElectionSupport.java | 41 +++++++-----
.../zookeeper/recipes/leader/LeaderOffer.java | 4 +-
zookeeper-recipes/zookeeper-recipes-lock/pom.xml | 9 ++-
.../apache/zookeeper/recipes/lock/WriteLock.java | 75 +++++++++++-----------
.../zookeeper/recipes/queue/DistributedQueue.java | 4 +-
zookeeper-server/pom.xml | 6 +-
10 files changed, 108 insertions(+), 63 deletions(-)
diff --git a/build.xml b/build.xml
index 5d1fc8f..d7edf7b 100644
--- a/build.xml
+++ b/build.xml
@@ -28,7 +28,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<!-- ====================================================== -->
<property name="slf4j.version" value="1.7.25"/>
<property name="commons-cli.version" value="1.2"/>
- <property name="spotbugsannotations.version" value="3.1.8"/>
+ <property name="spotbugsannotations.version" value="3.1.9"/>
<property name="wagon-http.version" value="2.4"/>
<property name="maven-ant-tasks.version" value="2.1.3"/>
diff --git a/pom.xml b/pom.xml
index 6d63041..e55e054 100755
--- a/pom.xml
+++ b/pom.xml
@@ -278,7 +278,8 @@
<kerby.version>1.1.0</kerby.version>
<bouncycastle.version>1.56</bouncycastle.version>
<commons-collections.version>3.2.2</commons-collections.version>
- <spotbugsannotations.version>3.1.8</spotbugsannotations.version>
+ <commons-lang.version>2.6</commons-lang.version>
+ <spotbugsannotations.version>3.1.9</spotbugsannotations.version>
</properties>
<dependencyManagement>
@@ -294,6 +295,11 @@
<version>${commons-collections.version}</version>
</dependency>
<dependency>
+ <groupId>commons-lang</groupId>
+ <artifactId>commons-lang</artifactId>
+ <version>${commons-lang.version}</version>
+ </dependency>
+ <dependency>
<groupId>org.apache.yetus</groupId>
<artifactId>audience-annotations</artifactId>
<version>${audience-annotations.version}</version>
@@ -472,7 +478,7 @@
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
- <version>3.1.8</version>
+ <version>3.1.9</version>
<configuration>
<excludeFilterFile>excludeFindBugsFilter.xml</excludeFilterFile>
</configuration>
diff --git a/zookeeper-contrib/pom.xml b/zookeeper-contrib/pom.xml
index 3ae211a..44bd725 100755
--- a/zookeeper-contrib/pom.xml
+++ b/zookeeper-contrib/pom.xml
@@ -34,11 +34,25 @@
<description>
Contrib projects to Apache ZooKeeper
</description>
-
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-maven-plugin</artifactId>
+ <version>3.1.9</version>
+ <configuration>
+ <!-- No spotbugs for contri modules -->
+ <skip>true</skip>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ </build>
<modules>
<module>zookeeper-contrib-loggraph</module>
<module>zookeeper-contrib-rest</module>
<module>zookeeper-contrib-zooinspector</module>
</modules>
-</project>
\ No newline at end of file
+</project>
diff --git a/zookeeper-recipes/pom.xml b/zookeeper-recipes/pom.xml
index 512e798..b92423a 100755
--- a/zookeeper-recipes/pom.xml
+++ b/zookeeper-recipes/pom.xml
@@ -62,4 +62,4 @@
<module>zookeeper-recipes-queue</module>
</modules>
-</project>
\ No newline at end of file
+</project>
diff --git
a/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderElectionSupport.java
b/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderElectionSupport.java
index 8d4096d..6013ffd 100644
---
a/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderElectionSupport.java
+++
b/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderElectionSupport.java
@@ -180,27 +180,36 @@ public class LeaderElectionSupport implements Watcher {
state = State.OFFER;
dispatchEvent(EventType.OFFER_START);
- leaderOffer = new LeaderOffer();
-
- leaderOffer.setHostName(hostName);
- leaderOffer.setNodePath(zooKeeper.create(rootNodeName + "/" + "n_",
- hostName.getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+ LeaderOffer newLeaderOffer = new LeaderOffer();
+ byte[] hostnameBytes;
+ synchronized (this) {
+ newLeaderOffer.setHostName(hostName);
+ hostnameBytes = hostName.getBytes();
+ newLeaderOffer.setNodePath(zooKeeper.create(rootNodeName + "/" + "n_",
+ hostnameBytes, ZooDefs.Ids.OPEN_ACL_UNSAFE,
CreateMode.EPHEMERAL_SEQUENTIAL));
-
+ leaderOffer = newLeaderOffer;
+ }
logger.debug("Created leader offer {}", leaderOffer);
dispatchEvent(EventType.OFFER_COMPLETE);
}
+ private synchronized LeaderOffer getLeaderOffer() {
+ return leaderOffer;
+ }
+
private void determineElectionStatus() throws KeeperException,
InterruptedException {
state = State.DETERMINE;
dispatchEvent(EventType.DETERMINE_START);
- String[] components = leaderOffer.getNodePath().split("/");
+ LeaderOffer currentLeaderOffer = getLeaderOffer();
+
+ String[] components = currentLeaderOffer.getNodePath().split("/");
- leaderOffer.setId(Integer.valueOf(components[components.length - 1]
+ currentLeaderOffer.setId(Integer.valueOf(components[components.length - 1]
.substring("n_".length())));
List<LeaderOffer> leaderOffers = toLeaderOffers(zooKeeper.getChildren(
@@ -215,7 +224,7 @@ public class LeaderElectionSupport implements Watcher {
for (int i = 0; i < leaderOffers.size(); i++) {
LeaderOffer leaderOffer = leaderOffers.get(i);
- if (leaderOffer.getId().equals(this.leaderOffer.getId())) {
+ if (leaderOffer.getId().equals(currentLeaderOffer.getId())) {
logger.debug("There are {} leader offers. I am {} in line.",
leaderOffers.size(), i);
@@ -237,7 +246,7 @@ public class LeaderElectionSupport implements Watcher {
throws KeeperException, InterruptedException {
logger.info("{} not elected leader. Watching node:{}",
- leaderOffer.getNodePath(), neighborLeaderOffer.getNodePath());
+ getLeaderOffer().getNodePath(), neighborLeaderOffer.getNodePath());
/*
* Make sure to pass an explicit Watcher because we could be sharing this
@@ -270,7 +279,7 @@ public class LeaderElectionSupport implements Watcher {
state = State.ELECTED;
dispatchEvent(EventType.ELECTED_START);
- logger.info("Becoming leader with node:{}", leaderOffer.getNodePath());
+ logger.info("Becoming leader with node:{}",
getLeaderOffer().getNodePath());
dispatchEvent(EventType.ELECTED_COMPLETE);
}
@@ -336,7 +345,7 @@ public class LeaderElectionSupport implements Watcher {
@Override
public void process(WatchedEvent event) {
if (event.getType().equals(Watcher.Event.EventType.NodeDeleted)) {
- if (!event.getPath().equals(leaderOffer.getNodePath())
+ if (!event.getPath().equals(getLeaderOffer().getNodePath())
&& state != State.STOP) {
logger.debug(
"Node {} deleted. Need to run through the election process.",
@@ -384,8 +393,8 @@ public class LeaderElectionSupport implements Watcher {
@Override
public String toString() {
- return "{ state:" + state + " leaderOffer:" + leaderOffer + " zooKeeper:"
- + zooKeeper + " hostName:" + hostName + " listeners:" + listeners
+ return "{ state:" + state + " leaderOffer:" + getLeaderOffer() + "
zooKeeper:"
+ + zooKeeper + " hostName:" + getHostName() + " listeners:" + listeners
+ " }";
}
@@ -437,11 +446,11 @@ public class LeaderElectionSupport implements Watcher {
* The hostname of this process. Mostly used as a convenience for logging and
* to respond to {@link #getLeaderHostName()} requests.
*/
- public String getHostName() {
+ public synchronized String getHostName() {
return hostName;
}
- public void setHostName(String hostName) {
+ public synchronized void setHostName(String hostName) {
this.hostName = hostName;
}
diff --git
a/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderOffer.java
b/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderOffer.java
index 188a6d5..bef634d 100644
---
a/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderOffer.java
+++
b/zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderOffer.java
@@ -16,6 +16,7 @@
*/
package org.apache.zookeeper.recipes.leader;
+import java.io.Serializable;
import java.util.Comparator;
/**
@@ -72,7 +73,8 @@ public class LeaderOffer {
* Compare two instances of {@link LeaderOffer} using only the {code}id{code}
* member.
*/
- public static class IdComparator implements Comparator<LeaderOffer> {
+ public static class IdComparator
+ implements Comparator<LeaderOffer>, Serializable {
@Override
public int compare(LeaderOffer o1, LeaderOffer o2) {
diff --git a/zookeeper-recipes/zookeeper-recipes-lock/pom.xml
b/zookeeper-recipes/zookeeper-recipes-lock/pom.xml
index ee9a796..f8ca267 100755
--- a/zookeeper-recipes/zookeeper-recipes-lock/pom.xml
+++ b/zookeeper-recipes/zookeeper-recipes-lock/pom.xml
@@ -41,6 +41,13 @@
<version>3.5.5-beta-SNAPSHOT</version>
</dependency>
<dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ <version>3.1.9</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+ <dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper-server</artifactId>
<version>3.5.5-beta-SNAPSHOT</version>
@@ -71,4 +78,4 @@
</plugins>
</build>
-</project>
\ No newline at end of file
+</project>
diff --git
a/zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/WriteLock.java
b/zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/WriteLock.java
index 5caebee..53f64cb 100644
---
a/zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/WriteLock.java
+++
b/zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/WriteLock.java
@@ -17,6 +17,7 @@
*/
package org.apache.zookeeper.recipes.lock;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.zookeeper.KeeperException;
@@ -85,7 +86,7 @@ public class WriteLock extends ProtocolSupport {
* return the current locklistener
* @return the locklistener
*/
- public LockListener getLockListener() {
+ public synchronized LockListener getLockListener() {
return this.callback;
}
@@ -93,7 +94,7 @@ public class WriteLock extends ProtocolSupport {
* register a different call back listener
* @param callback the call back instance
*/
- public void setLockListener(LockListener callback) {
+ public synchronized void setLockListener(LockListener callback) {
this.callback = callback;
}
@@ -133,8 +134,9 @@ public class WriteLock extends ProtocolSupport {
initCause(e);
}
finally {
- if (callback != null) {
- callback.lockReleased();
+ LockListener lockListener = getLockListener();
+ if (lockListener != null) {
+ lockListener.lockReleased();
}
id = null;
}
@@ -201,6 +203,8 @@ public class WriteLock extends ProtocolSupport {
* obtaining the lock
* @return if the command was successful or not
*/
+ @SuppressFBWarnings(value = "NP_NULL_PARAM_DEREF_NONVIRTUAL",
+ justification = "findPrefixInChildren will assign a value to
this.id")
public boolean execute() throws KeeperException, InterruptedException {
do {
if (id == null) {
@@ -211,41 +215,40 @@ public class WriteLock extends ProtocolSupport {
findPrefixInChildren(prefix, zookeeper, dir);
idName = new ZNodeName(id);
}
- if (id != null) {
- List<String> names = zookeeper.getChildren(dir, false);
- if (names.isEmpty()) {
- LOG.warn("No children in: " + dir + " when we've just
" +
- "created one! Lets recreate it...");
- // lets force the recreation of the id
- id = null;
- } else {
- // lets sort them explicitly (though they do seem to
come back in order ususally :)
- SortedSet<ZNodeName> sortedNames = new
TreeSet<ZNodeName>();
- for (String name : names) {
- sortedNames.add(new ZNodeName(dir + "/" + name));
+ List<String> names = zookeeper.getChildren(dir, false);
+ if (names.isEmpty()) {
+ LOG.warn("No children in: " + dir + " when we've just " +
+ "created one! Lets recreate it...");
+ // lets force the recreation of the id
+ id = null;
+ } else {
+ // lets sort them explicitly (though they do seem to come
back in order ususally :)
+ SortedSet<ZNodeName> sortedNames = new
TreeSet<ZNodeName>();
+ for (String name : names) {
+ sortedNames.add(new ZNodeName(dir + "/" + name));
+ }
+ ownerId = sortedNames.first().getName();
+ SortedSet<ZNodeName> lessThanMe =
sortedNames.headSet(idName);
+ if (!lessThanMe.isEmpty()) {
+ ZNodeName lastChildName = lessThanMe.last();
+ lastChildId = lastChildName.getName();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("watching less than me node: " +
lastChildId);
}
- ownerId = sortedNames.first().getName();
- SortedSet<ZNodeName> lessThanMe =
sortedNames.headSet(idName);
- if (!lessThanMe.isEmpty()) {
- ZNodeName lastChildName = lessThanMe.last();
- lastChildId = lastChildName.getName();
- if (LOG.isDebugEnabled()) {
- LOG.debug("watching less than me node: " +
lastChildId);
- }
- Stat stat = zookeeper.exists(lastChildId, new
LockWatcher());
- if (stat != null) {
- return Boolean.FALSE;
- } else {
- LOG.warn("Could not find the" +
- " stats for less than me: " +
lastChildName.getName());
- }
+ Stat stat = zookeeper.exists(lastChildId, new
LockWatcher());
+ if (stat != null) {
+ return Boolean.FALSE;
} else {
- if (isOwner()) {
- if (callback != null) {
- callback.lockAcquired();
- }
- return Boolean.TRUE;
+ LOG.warn("Could not find the" +
+ " stats for less than me: " +
lastChildName.getName());
+ }
+ } else {
+ if (isOwner()) {
+ LockListener lockListener = getLockListener();
+ if (lockListener != null) {
+ lockListener.lockAcquired();
}
+ return Boolean.TRUE;
}
}
}
diff --git
a/zookeeper-recipes/zookeeper-recipes-queue/src/main/java/org/apache/zookeeper/recipes/queue/DistributedQueue.java
b/zookeeper-recipes/zookeeper-recipes-queue/src/main/java/org/apache/zookeeper/recipes/queue/DistributedQueue.java
index c5d7c83..b636562 100644
---
a/zookeeper-recipes/zookeeper-recipes-queue/src/main/java/org/apache/zookeeper/recipes/queue/DistributedQueue.java
+++
b/zookeeper-recipes/zookeeper-recipes-queue/src/main/java/org/apache/zookeeper/recipes/queue/DistributedQueue.java
@@ -86,7 +86,7 @@ public class DistributedQueue {
continue;
}
String suffix = childName.substring(prefix.length());
- Long childId = new Long(suffix);
+ Long childId = Long.parseLong(suffix);
orderedChildren.put(childId,childName);
}catch(NumberFormatException e){
LOG.warn("Found child node with improper format : " +
childName + " " + e,e);
@@ -208,7 +208,7 @@ public class DistributedQueue {
}
}
- private class LatchChildWatcher implements Watcher {
+ private static class LatchChildWatcher implements Watcher {
CountDownLatch latch;
diff --git a/zookeeper-server/pom.xml b/zookeeper-server/pom.xml
index 12fe6ed..88a85d2 100755
--- a/zookeeper-server/pom.xml
+++ b/zookeeper-server/pom.xml
@@ -51,6 +51,10 @@
<scope>test</scope>
</dependency>
<dependency>
+ <groupId>commons-lang</groupId>
+ <artifactId>commons-lang</artifactId>
+ </dependency>
+ <dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper-jute</artifactId>
<version>3.5.5-beta-SNAPSHOT</version>
@@ -267,4 +271,4 @@
</plugins>
</build>
-</project>
\ No newline at end of file
+</project>
