Cache Node Assignment Service

[kx-chen]

Summary

This PR adds a basic draft of the cache node assignment service + tests. Several other changes were added in order to support the service, they include:

• Adding CacheNodeAssignment and CacheNodeMetadata classes (plus serializers, ZK store classes, proto file changes)
• Having the CachingChunkManager listen for changes in assignments and download data as appropriate

Requirements

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[mansu]

We already have code that does cache node assignment. How is this different from current logic?

It seems upon closer reading of the code, we are adding dynamic chunks. It would be good to clarify the design intent here.

If we want to support more chunks per cache node, why not increase the number of slots dynamically? Why do we need to change chunk assignment?

[kx-chen]

Hi Suman! Great to see you. Good question!

Yes, we are adding dynamic chunks. However, the goal is not just to support more chunks per cache node but also to improve disk space utilization.

I'm sure you already know, but chunks are not always a fixed size. While we aim for uniform size, they can vary. Since slots assume each chunk is the same size, this can lead to under-utilization of disk space if chunks are smaller than the assumed size.

For example, with a 3TB disk and 15GB per slot, we get 200 slots. If a chunk is only 10GB, there’s still 5GB of unused space allocated for that slot.

The change in assignment algorithm addresses this. We plan to use a bin-packing algorithm to pack all chunks into the least number of cache nodes, optimizing disk space usage and reducing costs.

Let me know if that answers your question! And apologies for the lack of context in the PR—it's still very much a work in progress 😅

[mansu]

Hi Kai, great to see you again!

Thanks for the context. It would ideal to write a design doc or a discussion for this before we start coding to document the goals of this change.

It seems that the goal of this change is improve disk utilization. This is a great goal. I am however curious why the there is so much wasted disk space unused in practice. If you are using size based roll over, at any given time only a small percentage of chunks would not be the same size as the cache slot but in steady state the chunks should be of the same size. If it is happening regularly, is it not a tuning issue? Also, even if the temporarily pack smaller chunks better(say after a deployment), would we not get back to a the same configuration as a fixed slot config once the roll over happens in an hour? I am asking since I may be missing a case where there are longer term advantages of this change.

Also, the current implementation of static number of slots results employs a constant work paradigm for the entire cluster. This makes cluster operations mechanical, predictable and easy to automate.

The dynamic slot model proposed here is much more dynamic and makes the system less predictable over all. This is big sacrifice from a design perspective.

To carefully evaluate the effect of these changes, it may be better to break this change down into a few smaller changes:

• The most basic feature needed here is ability to create dynamic slots on cache nodes instead of fixed slots.
• Once we can dynamically change number of slots, the next feature to consider is if we should allow nodes other than cache node(ex cluster manager) to create the slots.
• There may be a lot of ways in which we can optimize the cache assignment algorithm. So, we should probably make the assignment algorithm change independent of these dynamic nodes.

It is possible that we can get back disk utilization after the first 2 changes. It seems we are also changing the cache assignment nodes in this PR. I think that change may be better done independent of this change.

Overall, I think it may be ideal if we approach this change in a more incremental manner.

[bryanlb]

Looking pretty good! This is a pretty huge change, but you've done a good job of isolating the new behavior behind a "default false" system property.

I took a look over the code coverage as well, and it looks like you did a decent job of exercising the new paths. I think with a couple small tweaks this is probably good to merge up.

[bryanlb]

Couple nits, but I would consider addressing them in a future PR. I'm not seeing anything that sticks outs that we haven't discussed, and you have already successfully deployed this to several test environments.

[bryanlb]

Nit: These types of comments tend to go stale pretty quick, as folks refactor and miss a comment block several methods up when they're adding new methods.

[bryanlb]

Nit: This would be more useful if it contained information about what chunk was closed, otherwise this isn't much different than a metric.