Flat tree with binary-content nodes implementation of max-rate registry #1086
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cristaloleg
reviewed
Aug 16, 2019
...java/pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/ConsumerMaxRateRegistryType.java
Outdated
Show resolved
Hide resolved
jewertow
reviewed
Aug 19, 2019
hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/di/ConsumersBinder.java
Outdated
Show resolved
Hide resolved
...va/pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/FlatBinaryMaxRateRegistryTest.java
Outdated
Show resolved
Hide resolved
jewertow
reviewed
Aug 19, 2019
...n/java/pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/FlatBinaryMaxRateRegistry.java
Outdated
Show resolved
Hide resolved
cristaloleg
approved these changes
Aug 20, 2019
...main/java/pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/MaxRateRegistryFactory.java
Outdated
Show resolved
Hide resolved
...pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/HierarchicalCacheMaxRateRegistry.java
Outdated
Show resolved
Hide resolved
...pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/HierarchicalCacheMaxRateRegistry.java
Show resolved
Hide resolved
...pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/HierarchicalCacheMaxRateRegistry.java
Show resolved
Hide resolved
...ava/pl/allegro/tech/hermes/consumers/consumer/rate/maxrate/ConsumerRateHistoriesDecoder.java
Outdated
Show resolved
Hide resolved
cristaloleg
approved these changes
Aug 20, 2019
jewertow
approved these changes
Aug 21, 2019
cristaloleg
approved these changes
Aug 21, 2019
| && !maxRateRegistry.getMaxRate(new ConsumerInstance(consumerId, subscription2)).isPresent())); | ||
| } | ||
|
|
||
| @Test(expected = IllegalStateException.class) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing alternative implementation for max-rate registry in which we keep whole max-rate configuration dedicated to a single consumer in just one znode:
/hermes/consumers-rate/<cluster>/runtime-bin/<consumer-id>/max-rateOn the other hand consumer writes its current sending rate to a single znode:
/hermes/consumers-rate/<cluster>/runtime-bin/<consumer-id>/historyThese znodes are encoded in binary format (using SBE) and use numeric ids for subscription identification.
The leader node reads
historyznodes of all consumers on each max-rate computation. After the computations are done, it saves the results tomax-rateznodes, so consumers can update their sending max-rates.By using this registry implementation we generate much less znodes in zookeeper and need few zookeeper watches, the structure is flat and should load very fast and take not much space even for thousands of subscriptions controlled by a single consumer node. The number of znodes grows linearly as the number of consumers grow. I haven't implemented any zookeeper watches except a single
NodeCacheper consumer node, so it is immediately notified about max-rate being changed by the leader. Maybe in the future it will be worth implementing aPathChildrenCachejust for the leader node.HierarchicalCacheMaxRateRegistryis basically oldMaxRateRegistryclass, it's just renamed, asMaxRateRegistryis now an interface, so different implementations are available to choose from. Personally, I think the interface is too wide, but I wanted to leave it as is for now, so we don't change too much in the old max-rate implementation. If we stop supporting it, it will be easy to refactor and for example divide the registry into two parts: one for the leader that makes all the computations (so it reads history znodes of all consumers in its cluster), and second for all the nodes other nodes in which only max-rate znode is loaded and rate history znode is updated.Note: all classes from
pl.allegro.tech.hermes.consumers.consumer.rate.sbe.stubspackage are generated using SBE build tool via./gradlew hermes-consumers:generateSbeStubs.Schema configuration files that the stubs are generated from are kept in
hermes-consumers/src/main/resources/sbe/directory.