Limit the number of ephemeral nodes a session can create

[GrantPSpencer]

Description

In Zookeeper, an ephemeral node is only kept alive while the session it was created in is still active. When a session is closed by the client, all ephemeral nodes created by that session are deleted. Currently, there is no limit on how many ephemeral nodes can be created in a single session and if a large enough number of ephemeral nodes are created, then the work needed to delete these nodes and communicate the deletion to all followers can overwhelm the zookeeper server. This behavior has caused two previous Zookeeper site-up issues at LinkedIn.

My proposed solution to this problem is to throttle based on the cumulative bytes it takes to store all of the paths for the znodes created within that session. When a request comes into the PrepRequestProcessor, we check the DataTree and deny any create requests if we have already reached/exceeded the byte limit for that session.

With this approach, it is important to note that when a request is accepted, it would not immediately affect the ephemeral node count as it would need to reach the FinalRequestProcessor before the request affects the DataTree. This lag between the accepting ephemeral node requests and updating our ephemeral node count can lead to us not strictly upholding the limit. However, I believe this inaccuracy is acceptable as this should only result in minor variances of the total # of nodes allowed to be created and would still achieve the primary goal of preventing a server being overwhelmed when a session closes. If we wanted to be extra cautious, we could also fail requests at the end of the request processing pipeline when the new node is actually added to the DataTree.

4 years ago a PR (apache#1144) for a similar feature was opened against the opensource Zookeeper, but was never merged. The approach in that PR is slightly different as it attempts to keep a counter in the PrepRequestProcessor to track the # of ephemeral nodes currently in the DataTree. This solution does not have the same "lag time" problem as my proposed solution, but it does not account for the request failing at any step other than the PrepRequestProcessor. This could lead to the counter desyncing from the actual DataTree and being in an error state until reboot. Attempting to cover all the areas where a request could fail would also add quite a bit of constant work to the server for a feature that is intended to prevent an infrequent occurrence.

(Please let me know if this is not the correct repo/branch to target with these changes)

Tests

The following tests are written for this issue:
org/apache/zookeeper/server/quorum/EphemeralNodeThrottlingTest.java

The following is the result of the "mvn test" command on the appropriate module:

$ mvn test -o -Dtest=EphemeralNodeThrottlingTest -pl zookeeper-server/

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.zookeeper.server.quorum.EphemeralNodeThrottlingTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 14.93 s - in org.apache.zookeeper.server.quorum.EphemeralNodeThrottlingTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  21.543 s
[INFO] Finished at: 2023-09-25T13:57:39-07:00
[INFO] ------------------------------------------------------------------------

[rahulrane50]

Overall in right direction Grant! Thanks for picking it up.

[rahulrane50]

Excellent job @GrantPSpencer! Let's test this in EI once it's in!

[zpinto]

Nice work Grant! I left a few comments.

Also have one question. Why are we using the size of the path be the limit? Is it because we are worried about depth of tree of ephemeral nodes? If just ephemeral node count in general, counting each as 1 will be more efficient than iterating all chars in the path to get size.

[GrantPSpencer]

Nice work Grant! I left a few comments.

Also have one question. Why are we using the size of the path be the limit? Is it because we are worried about depth of tree of ephemeral nodes? If just ephemeral node count in general, counting each as 1 will be more efficient than iterating all chars in the path to get size.

We're using the sum of all the path sizes for that session as the limit because the closeSession transaction will contain all those znode paths. If the sum of those znode paths is larger than the jute max buffer, then the server will crash. I did look into ways to split up the closeSession transaction but did not come up with any viable solutions.

Previously I used the # of ephemeral znodes as a proxy, but Kiran pointed out that trying estimate the sum of the path sizes would be a more accurate way to track it

[zpinto]

LGTM, really nice work @GrantPSpencer!

[desaikomal]

You pivoted very fast from just count to using size. this is great change. just one minor comment.

[desaikomal]

Great change. Thanks @GrantPSpencer for working through this.