Repository: zookeeper Updated Branches: refs/heads/branch-3.4 6522d3f4d -> a378ea163
ZOOKEEPER-2733: Cleanup findbug warnings in branch-3.4: Dodgy code Warnings Author: Abraham Fine <[email protected]> Reviewers: Rakesh Radhakrishnan <[email protected]> Closes #236 from afine/ZOOKEEPER-2733 Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/a378ea16 Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/a378ea16 Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/a378ea16 Branch: refs/heads/branch-3.4 Commit: a378ea163b7110fa6f8aaf5fb07986d7901b1950 Parents: 6522d3f Author: Abraham Fine <[email protected]> Authored: Wed May 24 02:25:24 2017 -0700 Committer: Rakesh Radhakrishnan <[email protected]> Committed: Wed May 24 02:25:24 2017 -0700 ---------------------------------------------------------------------- .../zookeeper/server/PrepRequestProcessor.java | 9 ++++++--- .../apache/zookeeper/server/PurgeTxnLog.java | 15 +++++++++++---- .../zookeeper/server/SyncRequestProcessor.java | 2 +- .../server/persistence/FileTxnLog.java | 2 -- .../zookeeper/server/quorum/Follower.java | 2 ++ .../zookeeper/server/quorum/LearnerHandler.java | 2 +- .../zookeeper/server/quorum/Observer.java | 2 ++ .../quorum/auth/SaslQuorumAuthLearner.java | 10 ++-------- .../quorum/auth/SaslQuorumAuthServer.java | 2 +- .../zookeeper/server/upgrade/DataTreeV1.java | 17 ----------------- .../zookeeper/server/upgrade/UpgradeMain.java | 20 +++++++++++--------- src/java/test/config/findbugsExcludeFile.xml | 7 ++++++- 12 files changed, 43 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java index 58497b7..825c22a 100644 --- a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java +++ b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java @@ -505,6 +505,8 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements version = currentVersion + 1; request.txn = new CheckVersionTxn(path, version); break; + default: + LOG.error("Invalid OpCode: {} received by PrepRequestProcessor", type); } } @@ -588,9 +590,7 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements try { pRequest2Txn(op.getType(), zxid, request, subrequest, false); } catch (KeeperException e) { - if (ke == null) { - ke = e; - } + ke = e; request.hdr.setType(OpCode.error); request.txn = new ErrorTxn(e.code().intValue()); LOG.info("Got user-level KeeperException when processing " @@ -641,6 +641,9 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements zks.sessionTracker.checkSession(request.sessionId, request.getOwner()); break; + default: + LOG.warn("unknown type " + request.type); + break; } } catch (KeeperException e) { if (request.hdr != null) { http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java b/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java index d4effaf..11112a4 100644 --- a/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java +++ b/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java @@ -133,11 +133,18 @@ public class PurgeTxnLog { } } // add all non-excluded log files - List<File> files = new ArrayList<File>(Arrays.asList(txnLog - .getDataDir().listFiles(new MyFileFilter(PREFIX_LOG)))); + List<File> files = new ArrayList<File>(); + File[] fileArray = txnLog.getDataDir().listFiles(new MyFileFilter(PREFIX_LOG)); + if (fileArray != null) { + files.addAll(Arrays.asList(fileArray)); + } + // add all non-excluded snapshot files to the deletion list - files.addAll(Arrays.asList(txnLog.getSnapDir().listFiles( - new MyFileFilter(PREFIX_SNAPSHOT)))); + fileArray = txnLog.getSnapDir().listFiles(new MyFileFilter(PREFIX_SNAPSHOT)); + if (fileArray != null) { + files.addAll(Arrays.asList(fileArray)); + } + // remove the old files for(File f: files) { http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java index f622b24..317b4ac 100644 --- a/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java +++ b/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java @@ -140,7 +140,7 @@ public class SyncRequestProcessor extends ZooKeeperCriticalThread implements Req if (zks.getZKDatabase().append(si)) { logCount++; if (logCount > (snapCount / 2 + randRoll)) { - randRoll = r.nextInt(snapCount/2); + setRandRoll(r.nextInt(snapCount/2)); // roll the log zks.getZKDatabase().rollLog(); // take a snapshot http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java index b1682c3..690cfca 100644 --- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java +++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java @@ -637,8 +637,6 @@ public class FileTxnLog implements TxnLog { crc.update(bytes, 0, bytes.length); if (crcValue != crc.getValue()) throw new IOException(CRC_ERROR); - if (bytes == null || bytes.length == 0) - return false; hdr = new TxnHeader(); record = SerializeUtils.deserializeTxn(bytes, hdr); } catch (EOFException e) { http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/Follower.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/quorum/Follower.java b/src/java/main/org/apache/zookeeper/server/quorum/Follower.java index e439aaa..27c3375 100644 --- a/src/java/main/org/apache/zookeeper/server/quorum/Follower.java +++ b/src/java/main/org/apache/zookeeper/server/quorum/Follower.java @@ -136,6 +136,8 @@ public class Follower extends Learner{ case Leader.SYNC: fzk.sync(); break; + default: + LOG.error("Invalid packet type: {} received by Observer", qp.getType()); } } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java b/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java index 51ed7e7..4820490 100644 --- a/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java +++ b/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java @@ -272,7 +272,7 @@ public class LearnerHandler extends ZooKeeperThread { type = "PROPOSAL"; TxnHeader hdr = new TxnHeader(); try { - txn = SerializeUtils.deserializeTxn(p.getData(), hdr); + SerializeUtils.deserializeTxn(p.getData(), hdr); // mess = "transaction: " + txn.toString(); } catch (IOException e) { LOG.warn("Unexpected exception",e); http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/Observer.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/quorum/Observer.java b/src/java/main/org/apache/zookeeper/server/quorum/Observer.java index 53f516f..ded4a24 100644 --- a/src/java/main/org/apache/zookeeper/server/quorum/Observer.java +++ b/src/java/main/org/apache/zookeeper/server/quorum/Observer.java @@ -125,6 +125,8 @@ public class Observer extends Learner{ ObserverZooKeeperServer obs = (ObserverZooKeeperServer)zk; obs.commitRequest(request); break; + default: + LOG.error("Invalid packet type: {} received by Observer", qp.getType()); } } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java index 9a38bb8..8c76d3e 100644 --- a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java +++ b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java @@ -173,14 +173,8 @@ public class SaslQuorumAuthLearner implements QuorumAuthLearner { BufferedOutputStream bufferedOutput = new BufferedOutputStream(dout); BinaryOutputArchive boa = BinaryOutputArchive .getArchive(bufferedOutput); - if (response == null) { - authPacket = QuorumAuth.createPacket( - QuorumAuth.Status.IN_PROGRESS, response); - } else { - authPacket = QuorumAuth.createPacket( - QuorumAuth.Status.IN_PROGRESS, response); - } - + authPacket = QuorumAuth.createPacket( + QuorumAuth.Status.IN_PROGRESS, response); boa.writeRecord(authPacket, QuorumAuth.QUORUM_AUTH_MESSAGE_TAG); bufferedOutput.flush(); } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java index a64513a..1aa2b32 100644 --- a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java +++ b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java @@ -169,7 +169,7 @@ public class SaslQuorumAuthServer implements QuorumAuthServer { QuorumAuthPacket authPacket; if (challenge == null && s != QuorumAuth.Status.SUCCESS) { authPacket = QuorumAuth.createPacket( - QuorumAuth.Status.IN_PROGRESS, challenge); + QuorumAuth.Status.IN_PROGRESS, null); } else { authPacket = QuorumAuth.createPacket(s, challenge); } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java b/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java index e3d0633..678f4c6 100644 --- a/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java +++ b/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java @@ -347,14 +347,6 @@ public class DataTreeV1 { public long zxid; - public int err; - - public int type; - - public String path; - - public Stat stat; - /** * Equality is defined as the clientId and the cxid being the same. This * allows us to use hash tables to track completion of transactions. @@ -394,8 +386,6 @@ public class DataTreeV1 { rc.clientId = header.getClientId(); rc.cxid = header.getCxid(); rc.zxid = header.getZxid(); - rc.type = header.getType(); - rc.err = 0; if (rc.zxid > lastProcessedZxid) { lastProcessedZxid = rc.zxid; } @@ -406,7 +396,6 @@ public class DataTreeV1 { createNode(createTxn.getPath(), createTxn.getData(), createTxn .getAcl(), createTxn.getEphemeral() ? header .getClientId() : 0, header.getZxid(), header.getTime()); - rc.path = createTxn.getPath(); break; case OpCode.delete: DeleteTxn deleteTxn = (DeleteTxn) txn; @@ -416,22 +405,16 @@ public class DataTreeV1 { case OpCode.setData: SetDataTxn setDataTxn = (SetDataTxn) txn; debug = "Set data for transaction for " + setDataTxn.getPath(); - rc.stat = setData(setDataTxn.getPath(), setDataTxn.getData(), - setDataTxn.getVersion(), header.getZxid(), header - .getTime()); break; case OpCode.setACL: SetACLTxn setACLTxn = (SetACLTxn) txn; debug = "Set ACL for transaction for " + setACLTxn.getPath(); - rc.stat = setACL(setACLTxn.getPath(), setACLTxn.getAcl(), - setACLTxn.getVersion()); break; case OpCode.closeSession: killSession(header.getClientId()); break; case OpCode.error: ErrorTxn errTxn = (ErrorTxn) txn; - rc.err = errTxn.getErr(); break; } } catch (KeeperException e) { http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java b/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java index 779c481..6648a12 100644 --- a/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java +++ b/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java @@ -114,15 +114,17 @@ public class UpgradeMain { */ void copyFiles(File srcDir, File dstDir, String filter) throws IOException { File[] list = srcDir.listFiles(); - for (File file: list) { - String name = file.getName(); - if (name.startsWith(filter)) { - // we need to copy this file - File dest = new File(dstDir, name); - LOG.info("Renaming " + file + " to " + dest); - if (!file.renameTo(dest)) { - throw new IOException("Unable to rename " - + file + " to " + dest); + if (list != null) { + for (File file: list) { + String name = file.getName(); + if (name.startsWith(filter)) { + // we need to copy this file + File dest = new File(dstDir, name); + LOG.info("Renaming " + file + " to " + dest); + if (!file.renameTo(dest)) { + throw new IOException("Unable to rename " + + file + " to " + dest); + } } } } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/test/config/findbugsExcludeFile.xml ---------------------------------------------------------------------- diff --git a/src/java/test/config/findbugsExcludeFile.xml b/src/java/test/config/findbugsExcludeFile.xml index 7c6c532..de8e078 100644 --- a/src/java/test/config/findbugsExcludeFile.xml +++ b/src/java/test/config/findbugsExcludeFile.xml @@ -96,7 +96,12 @@ <Match> <Class name="org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerSender"/> <Method name="process"/> - <Bug code="RV"/> + <Bug code="RV,SF"/> + </Match> + <Match> + <Class name="org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerReceiver"/> + <Method name="run"/> + <Bug code="SF"/> </Match> <!-- these are old classes just for upgrading and should go away -->
