Initial metadata store trait specification

[tillrohrmann]

Initial metadata store trait specification

[AhmedSoliman]

Looks neat. I'm curious to know your plan on how will we manage putting the version value in (and out) of the metadata value itself (e.g. nodes_config's version(), etc.)

[AhmedSoliman]

We might also need a set() that always overwrites (accepts a key, value, version) for admin operations.

[tillrohrmann]

This is needed if the version of the payload is the version of the stored key-value pair, right? In case we separate those two, put w/o a precondition could probably already work.

[AhmedSoliman]

I was assuming that we'd want to keep them in sync, no? What's the motivation for maintaining two versions?

[tillrohrmann]

I think it would only make sense to have two versions if we wanted to store values that don't have an inherent version. At the moment, I think all our metadata is versioned so it does not make sense for the metadata. Will the cluster controller store state that is not versioned (like the latest plan how to split and move partitions)? If this is not needed, then I could change the trait to take a Versioned + Serialize value.

[AhmedSoliman]

I'm assuming that everything will be versioned, but even if not, wouldn't a set() API work for those (assuming that they'd always use a fixed version)?

[AhmedSoliman]

Also, does the trait itself need to be Send+Sync?

[tillrohrmann]

Also, does the trait itself need to be Send+Sync?

You are right, it is not needed.

I'm assuming that everything will be versioned, but even if not, wouldn't a set() API work for those (assuming that they'd always use a fixed version)?

If the value would use a fixed version then it will become harder to tell what is the valid value if there are two writers (e.g. 2 cluster controllers that try to do contradicting operations). But I guess the answer would then be to add an explicit version to all state that is stored in the MetadataStore.

[tillrohrmann]

Looks neat. I'm curious to know your plan on how will we manage putting the version value in (and out) of the metadata value itself (e.g. nodes_config's version(), etc.)

So far I was thinking of treating the version of the metadata store key-value pair independent of the version of a potentially versioned value (e.g. nodes configuration, schema information, etc.). This adds a bit more metadata and I guess slightly more complex client-side implementation. On the other hand, it does not require that values that are being stored have a version themselves.

[AhmedSoliman]

So far I was thinking of treating the version of the metadata store key-value pair independent of the version of a potentially versioned value (e.g. nodes configuration, schema information, etc.). This adds a bit more metadata and I guess slightly more complex client-side implementation. On the other hand, it does not require that values that are being stored have a version themselves.

It should be possible to synchronize both copies of the version, but this means an implicit assumption that metadata store will always return the version+1, right?

[tillrohrmann]

It should be possible to synchronize both copies of the version, but this means an implicit assumption that metadata store will always return the version+1, right?

Yes, I think so. In my current implementation, this is the case because of my shameless reusing of the Version struct but it is not strictly required right now. I could add this as a requirement for all MetadataStore implementations.

[AhmedSoliman]

Also, does the trait itself need to be Send+Sync?

[tillrohrmann]

I've added a commit outlining how the trait with explicit version propagation could look like.

[AhmedSoliman]

🍾 good to go after making linter happy and fixing the mismatching comment.

[tillrohrmann]

Thanks for the swift review @AhmedSoliman :-) Merging this PR now.