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

Cluster peer discovery improvements and fixes #1387

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

thampiotr
Copy link
Contributor

@thampiotr thampiotr commented Jul 31, 2024

PR Description

Continuation of #1394.

  • Clustering peer resolution through --cluster.join-addresses flag has been
    improved with more consistent behaviour, better error handling and added
    support for A/AAAA DNS records. If necessary, users can temporarily opt out of
    this new behaviour with the --cluster.use-discovery-v1, but this can only be
    used as a temporary measure, since this flag will be disabled in future
    releases.

  • Fixed an issue where providing multiple hostnames or IP addresses
    via --cluster.join-addresses would only use the first provided value.

  • Fixed an issue where providing <hostname>:<port>
    in --cluster.join-addresses would only resolve with DNS to a single address,
    instead of using all the available records.

  • Fixed an issue where clustering peers resolution via hostname in --cluster.join-addresses
    resolves to duplicated IP addresses when using SRV records.

  • Added more tests to cover the above

  • Refactored for readability (separate commit from test changes, verified tests pass)

  • Improved error handling, logging and added tracing.

Which issue(s) this PR fixes

#1208

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated

@thampiotr thampiotr force-pushed the thampiotr/cluster-dns-queries-v2 branch 2 times, most recently from 920382e to d722910 Compare July 31, 2024 13:46
Base automatically changed from thampiotr/cluster-dns-queries-v2 to main July 31, 2024 14:19
@thampiotr thampiotr force-pushed the thampiotr/cluster-discovery-v2-changes branch 2 times, most recently from f59b725 to 5dc6265 Compare July 31, 2024 14:26
@thampiotr thampiotr changed the title Refactor cluster peer discovery Cluster peer discovery v2 Jul 31, 2024
@thampiotr thampiotr force-pushed the thampiotr/cluster-discovery-v2-changes branch 4 times, most recently from 533534e to 5b4981e Compare July 31, 2024 17:29
@thampiotr thampiotr changed the base branch from main to revert-1390-revert-1311-thampiotr/cluster-dns-queries-v2 July 31, 2024 17:32
@thampiotr thampiotr force-pushed the thampiotr/cluster-discovery-v2-changes branch from 5b4981e to cf0e5d2 Compare August 1, 2024 14:57
Base automatically changed from revert-1390-revert-1311-thampiotr/cluster-dns-queries-v2 to main August 2, 2024 10:25
@thampiotr thampiotr force-pushed the thampiotr/cluster-discovery-v2-changes branch from cf0e5d2 to d29900b Compare August 5, 2024 13:33
@thampiotr thampiotr marked this pull request as ready for review August 6, 2024 11:57
@thampiotr thampiotr changed the title Cluster peer discovery v2 Cluster peer discovery improvements and fixes Aug 6, 2024
@mattdurham
Copy link
Collaborator

Will put some eyes on this tomorrow.

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Overall a few questions but overall good.

internal/service/cluster/discovery/join_peers.go Outdated Show resolved Hide resolved
internal/service/cluster/discovery/join_peers.go Outdated Show resolved Hide resolved
@thampiotr thampiotr force-pushed the thampiotr/cluster-discovery-v2-changes branch from 3a0133b to 5b991c6 Compare August 9, 2024 13:06
@thampiotr thampiotr enabled auto-merge (squash) August 9, 2024 13:07
@thampiotr thampiotr merged commit 0499453 into main Aug 9, 2024
18 checks passed
@thampiotr thampiotr deleted the thampiotr/cluster-discovery-v2-changes branch August 9, 2024 13:11
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants