[GitHub] zookeeper pull request #563: ZOOKEEPER-3072: Throttle race condition fix

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/563


---

[GitHub] zookeeper pull request #563: ZOOKEEPER-3072: Throttle race condition fix

[bothejjms]

Github user bothejjms commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/563#discussion_r205778339
  
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -1128,9 +1128,9 @@ public void processPacket(ServerCnxn cnxn, ByteBuffer 
incomingBuffer) throws IOE
 Record rsp = processSasl(incomingBuffer,cnxn);
 ReplyHeader rh = new ReplyHeader(h.getXid(), 0, 
KeeperException.Code.OK.intValue());
 cnxn.sendResponse(rh,rsp, "response"); // not sure about 
3rd arg..what is it?
-return;
--- End diff --

I have refactored like that.
Returns are actually unnecessary but I have consistently added them now.


---

[GitHub] zookeeper pull request #563: ZOOKEEPER-3072: Throttle race condition fix

[breed]

Github user breed commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/563#discussion_r205668951
  
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -1128,9 +1128,9 @@ public void processPacket(ServerCnxn cnxn, ByteBuffer 
incomingBuffer) throws IOE
 Record rsp = processSasl(incomingBuffer,cnxn);
 ReplyHeader rh = new ReplyHeader(h.getXid(), 0, 
KeeperException.Code.OK.intValue());
 cnxn.sendResponse(rh,rsp, "response"); // not sure about 
3rd arg..what is it?
-return;
--- End diff --

it would be nice to keep this return since it matches the handling of the 
other auth logic above.

it would also be nice if this was an

`} else if (h.getType() == OpCode.sasl) {`

clause and the
`}
else {`

was done outside of the if since all the other blocks will have returned. i 
think it makes the logic easier to follow.


---