jhuan31 commented on a change in pull request #986: ZOOKEEPER-3243: Add
server-side request throttling
URL: https://github.com/apache/zookeeper/pull/986#discussion_r299194984
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
##########
@@ -229,6 +243,19 @@ public void setStale() {
stale = true;
}
+ public boolean isInvalid() {
+ return invalid;
+ }
+
+ public void setInvalid() {
+ if (!invalid) {
+ if (!stale) {
Review comment:
This is a tricky one. We went through the code and had a discussion. We're
not sure whether calling sendCloseSession() when stale is set could help solve
the "false positive" you saw--setStale() is only called from the close()
function so closing must have been initiated already when stale is set. We do
come up with an alternative solution: instead of calling sendCloseSession() in
setInvalid(), we could call sendCloseSession() when dropping a request and set
the invalid flag within sendCloseSession() so we could make sure that close
connection is sent to the client. This solution seems simple and *should* work
but it is not tested. The current implementation has been in production for 2+
years so we're inclined to stay put. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
