ZOOKEEPER-3683: Discard requests that are delayed longer than a confi…

[jhuan31]

…gured threshold

[ztzg]

LGTM, in general. (But "Request Changes" because System.exit won't pass "checkstyle" as-is anyway.)

[ztzg]

LGTM!

[ztzg]

Hi @jhuan31,

I am very interested in "multitenancy" topics such as client identification, throttling and quotas, and notably in the components of this ticket submitted by @gmcatsf:

ZOOKEEPER-3467, Complete quota system for zookeeper server

It seems there is a constant stream of patches coming from Facebook which progressively add missing bits and pieces. This is great, but it also means that any new work in that area is likely to duplicate something you already have working internally.

So I am wondering: would you, per chance, have a rough list of "features" you are planning to upstream, so that we can coordinate and minimize conflicts/duplication?

Thanks, -D

[eolivelli]

@ztzg yes, ZookKeeper Friends @ Facebook are porting most of their internal changes to Apache ZooKeeper.
They are creating JIRAs.

If you check on the ML they are hosting a meetup this month in order to present their work. They did the same some month ago.

It is a great chance for the community to benefit from their experience as they are running ZooKeeper at scale.

[ztzg]

@ztzg yes, ZookKeeper Friends @ Facebook are porting most of their internal changes to Apache ZooKeeper. They are creating JIRAs.

Great :)

If you check on the ML they are hosting a meetup this month in order to present their work. They did the same some month ago.

Right; I should have mentioned that. I won't be able to attend (I'm in Germany), at least this time, but am planning to try and follow the live stream. But I now see that there are recordings of the previous sessions; I will check those!

It is a great chance for the community to benefit from their experience as they are running ZooKeeper at scale.

Indeed!

Cheers, -D

[anmolnar]

@eolivelli @jhuan31

Theoretic question: this is a very useful and huge patch from FB. We discussed that 3.6.0 will be the "Facebook" release which means contains all patches which FB folks wanted to upstream. So, question for committers: can we submit this to branch-3.6 once accepted?

[eolivelli]

branch-3.6 is almost stable and it should be put into only bugfix-and-minor-improvements-mode as soon as 3.6.0 is out.

This patch is very big and It is also adding new RCs (ThrottledOpException)

I would go for 3.7.

We already got into troubles due to the multi address features committed right before the release :-)

[anmolnar]

@eolivelli I understand just saying we had some committments and I feel we started releasing 3.6 a little bit too early. Though it's also my fault: I haven't shout against cutting 3.6

[lvfangmin]

This feature is not fully enabled on our prod environment, I think it's fine to go for 3.7 given we already have a lot of reliability and cool features added in 3.6.

Any other feedback of this diff, or we can go ahead and merge it?

[hanm]

requestThrottleQueueTime might require some changes - other parts lgtm with some nits on logging and doc.

[jhuan31]

@hanm Thank you very much for your comments. I've addressed them. Sorry about the delay.

[jhuan31]

retest maven build

[ztzg]

@hanm, re requestThrottleQueueTime: Nice catch!