-
Notifications
You must be signed in to change notification settings - Fork 92
đ (ZENKO-5303) make voting and priority right #2452
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
base: development/2.16
Are you sure you want to change the base?
Changes from all commits
e4d966a
cd8a868
b7d315f
ae35701
ca209b3
78c5b85
45ff404
0495d99
4dccbb3
9c9a549
28ef5f1
df74ca4
bee96a4
b3a2b3f
09768f7
8888006
6a005ee
65bbbc3
80a2b7c
9445688
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
DarkIsDude marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -910,14 +910,17 @@ mongodb_is_secondary_node_ready() { | |
| local -r port="${2:?port is required}" | ||
|
|
||
| debug "Waiting for the node to be marked as secondary" | ||
| # mongosh prints a connection banner holding 'directConnection=true', so matching | ||
| # on 'true' would always succeed. Match a dedicated sentinel instead, built at | ||
| # runtime so the positive value never appears verbatim in the submitted script. | ||
| result=$( | ||
| mongodb_execute_print_output "$MONGODB_INITIAL_PRIMARY_ROOT_USER" "$MONGODB_INITIAL_PRIMARY_ROOT_PASSWORD" "admin" "$MONGODB_INITIAL_PRIMARY_HOST" "$MONGODB_INITIAL_PRIMARY_PORT_NUMBER" <<EOF | ||
| rs.status().members.filter(m => m.name === '$node:$port' && m.stateStr === 'SECONDARY').length === 1 | ||
| print("IS_SECONDARY_" + (rs.status().members.some(m => m.name === '$node:$port' && m.stateStr === 'SECONDARY') ? "YES" : "NO")) | ||
| EOF | ||
| ) | ||
| debug "$result" | ||
|
|
||
| grep -q "true" <<<"$result" | ||
| grep -q "IS_SECONDARY_YES" <<<"$result" | ||
| } | ||
|
|
||
| ######################## | ||
|
|
@@ -951,6 +954,35 @@ EOF | |
| grep -q "ok: 1" <<<"$result" | ||
| } | ||
|
|
||
| ######################## | ||
| # Get if secondary node already has voting rights | ||
|
DarkIsDude marked this conversation as resolved.
|
||
| # Globals: | ||
| # MONGODB_* | ||
| # Arguments: | ||
| # $1 - node | ||
| # $2 - port | ||
| # Returns: | ||
| # Boolean | ||
| ######################### | ||
| mongodb_secondary_node_has_voting_rights() { | ||
| local -r node="${1:?node is required}" | ||
| local -r port="${2:?port is required}" | ||
| local result | ||
|
|
||
| debug "Checking voting rights of the node" | ||
| # mongosh prints a connection banner holding 'directConnection=true', so matching | ||
| # on 'true' would always succeed. Match a dedicated sentinel instead, built at | ||
| # runtime so the positive value never appears verbatim in the submitted script. | ||
| result=$( | ||
| mongodb_execute_print_output "$MONGODB_INITIAL_PRIMARY_ROOT_USER" "$MONGODB_INITIAL_PRIMARY_ROOT_PASSWORD" "admin" "$MONGODB_INITIAL_PRIMARY_HOST" "$MONGODB_INITIAL_PRIMARY_PORT_NUMBER" <<EOF | ||
| print("HAS_VOTES_" + (rs.conf().members.some(m => m.host === '$node:$port' && m.votes > 0 && m.priority > 0) ? "YES" : "NO")) | ||
| EOF | ||
| ) | ||
| debug "$result" | ||
|
|
||
| grep -q "HAS_VOTES_YES" <<<"$result" | ||
| } | ||
|
|
||
| ######################## | ||
| # Get if hidden node is pending | ||
| # Globals: | ||
|
|
@@ -1187,7 +1219,15 @@ mongodb_configure_secondary() { | |
| exit 1 | ||
| fi | ||
| mongodb_wait_confirmation "$node" "$port" | ||
| fi | ||
|
|
||
| # Grant voting rights to the node if it does not have them yet. This must be | ||
| # done even when the node is already in the cluster: a previous attempt may | ||
| # have added it with votes/priority 0 and failed (or been restarted) before | ||
| # granting voting rights, leaving the node stuck without them. | ||
| if mongodb_secondary_node_has_voting_rights "$node" "$port"; then | ||
| info "Node already has voting rights" | ||
| else | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will not behave properly when we have 9-replicas, where 2 nodes are always "non voting". This is expected, and we must not try to change that (in particular, the voting nodes must be in precisely the expected DC for proper redundancy) â the change will make the startup slower on these extra secondaries -(re-)trying to make them voting-, possibly with the extra risk of changing the set of voters...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DarkIsDude did you try that case? Looking at the code below, it seems that the script would exit if it fails to configure secondary node voting: does that function already silently handle the case where we reached the max number of voters already?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes â tested on a real 9-replica shard, both with the current development/2.15 image and with this branch. Short answer to your question: no, it is not silently handled â it exits. But that is pre-existing behaviour, not something this PR introduces. First: your review caught a real bug, just not the one we were discussing. Secondly: "the extra risk of changing the set of voters": The only path where this branch differs from development/2.15 is data dir empty and node already in rs.conf() â i.e. PVC loss on an existing member. Last one: "does that function silently handle reaching the max number of voters?" No. Reproduced, scaling 3 â 9: It does recover: on restart the data dir is no longer empty, config is skipped, and the member settles as a working non-voting secondary. Net cost is one crash + ~3 min per extra member, and the final topology is the expected 9 members / 7 voters.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ack for the unchanged behavior, but I guess it's best to fix it, right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we should fix it.
If we really wants to fix it, we should read the output from Mongo, parse it and stop retrying. Again, not sure is useful right now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please create a followup BUGFIX ticket (in Zenko) to document the problem, and we may pick it up (or not) later.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see this is resolved. What's the followup ticket?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bump |
||
| # Ensure that secondary nodes do not count as voting members until they are fully initialized | ||
| # https://docs.mongodb.com/manual/reference/method/rs.add/#behavior | ||
| if ! retry_while "mongodb_is_secondary_node_ready $node $port" "$MONGODB_INIT_RETRY_ATTEMPTS" "$MONGODB_INIT_RETRY_DELAY"; then | ||
|
|
@@ -1198,17 +1238,16 @@ mongodb_configure_secondary() { | |
| # Grant voting rights to node | ||
| # https://docs.mongodb.com/manual/tutorial/modify-psa-replica-set-safely/ | ||
| if ! retry_while "mongodb_configure_secondary_node_voting $node $port" "$MONGODB_INIT_RETRY_ATTEMPTS" "$MONGODB_INIT_RETRY_DELAY"; then | ||
| error "Secondary node did not get marked as secondary" | ||
| error "Secondary node did not get granted voting rights" | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| # Mark node as readable. This is necessary in cases where the PVC is lost | ||
| if is_boolean_yes "$MONGODB_SET_SECONDARY_OK"; then | ||
| mongodb_execute_print_output "$MONGODB_INITIAL_PRIMARY_ROOT_USER" "$MONGODB_INITIAL_PRIMARY_ROOT_PASSWORD" "admin" <<EOF | ||
| # Mark node as readable. This is necessary in cases where the PVC is lost | ||
| if is_boolean_yes "$MONGODB_SET_SECONDARY_OK"; then | ||
| mongodb_execute_print_output "$MONGODB_INITIAL_PRIMARY_ROOT_USER" "$MONGODB_INITIAL_PRIMARY_ROOT_PASSWORD" "admin" <<EOF | ||
| rs.secondaryOk() | ||
| EOF | ||
| fi | ||
|
|
||
| fi | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.