Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NodeState to ShotoverNode + some renaming for KafkaSinkCluster #1758

Merged

Conversation

justinweng-instaclustr
Copy link
Collaborator

@justinweng-instaclustr justinweng-instaclustr commented Sep 30, 2024

  • Add a new ShotoverNodeState field to ShotoverNode. We are not reusing the existing NodeState for Kafka because it's possible that we will have different node states for Shotover in the future.
  • Add a new configuration check_shotover_peers_delay_ms to KafkaSinkCluster which will be used when checking connection to nodes and setting node states.
  • Move ShotoverNode code into a new file kafka/sink_cluster/shotover_node.rs
  • Rename kafka/sink_cluster/node.rs to kafka/sink_cluster/kafka_node.rs and NodeState to KafkaNodeState

Copy link

codspeed-hq bot commented Sep 30, 2024

CodSpeed Performance Report

Merging #1758 will not alter performance

Comparing justinweng-instaclustr:handle-offline-shotover-nodes (f45682c) with main (25827de)

Summary

✅ 39 untouched benchmarks

@justinweng-instaclustr justinweng-instaclustr marked this pull request as ready for review September 30, 2024 07:00
Copy link
Member

@rukai rukai left a comment

Choose a reason for hiding this comment

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

Looks good!

@conorbros conorbros merged commit eb4fb2f into shotover:main Oct 1, 2024
41 checks passed
@justinweng-instaclustr justinweng-instaclustr changed the title Add NodeState to ShotoverNode + some renaming Add NodeState to ShotoverNode + some renaming for KafkaSinkCluster Oct 8, 2024
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