Skip to content

Conversation

@olim7t
Copy link
Contributor

@olim7t olim7t commented Mar 5, 2019

With JAVA-2165, the Java driver will now index its node metadata by host_id. This won't work with Simulacron's default behavior, which is to assign the same UUID to every node. We need unique ids.

This is the simplest way to address the issue. It fixes my problem but is a bit fragile if the user overrides the key manually, or the whole map by calling withPeerInfo(Map). Maybe we should check that a host_id is always present, or even promote it to a top-level property like withCassandraVersion.

@olim7t
Copy link
Contributor Author

olim7t commented Mar 5, 2019

Mmm I've started working on the test failures, and I realize my solution is not great because it also creates a host_id in the cluster-level peer info. I'll wait for your advice if you think of a better approach.

@tolbertam
Copy link
Contributor

promote it to a top-level property like withCassandraVersion.

I like this idea. If host_id is getting more precedence in the drivers it makes sense to promote it as a node-level attribute that defaults a random id and can be overridden by user if needed and the PeerMetadataHandler can pull from this attribute.

@olim7t
Copy link
Contributor Author

olim7t commented Mar 5, 2019

OK I'll work on those changes.

@olim7t olim7t force-pushed the unique_host_ids branch from 1005736 to ec539f8 Compare March 6, 2019 00:02
@olim7t
Copy link
Contributor Author

olim7t commented Mar 6, 2019

Done. I force-pushed since there was nothing in common with my initial attempt.

Copy link
Contributor

@tolbertam tolbertam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks excellent, thanks! I'll merge and release this today

@tolbertam tolbertam merged commit 6d33341 into master Mar 6, 2019
@tolbertam tolbertam deleted the unique_host_ids branch March 6, 2019 15:37
@tolbertam tolbertam added this to the 0.8.8 milestone Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants