Skip to content

Add PushPullNodes config option#11

Merged
pracucci merged 2 commits intomasterfrom
push-pull-n-nodes
Nov 25, 2025
Merged

Add PushPullNodes config option#11
pracucci merged 2 commits intomasterfrom
push-pull-n-nodes

Conversation

@pracucci
Copy link

@pracucci pracucci commented Nov 25, 2025

Description

In this PR I'm adding a new config option to configure how many nodes should be contacted for each push/pull cycle. The default is 1, which is the previous behaviour, but allows to increase it.

This change will be used in conjunction with the zone-aware routing, to have bridge nodes doing push/pull to 2 nodes per cycle. See a more detailed explanation here, where I use the new feature.

Related Issue

Ref: https://github.com/grafana/mimir-squad/issues/2634

How Has This Been Tested?

See: grafana/dskit#798

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Copy link

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

LGTM

@pracucci pracucci merged commit 2125cd9 into master Nov 25, 2025
41 of 45 checks passed
pracucci added a commit to grafana/dskit that referenced this pull request Nov 25, 2025
…cle (#798)

**What this PR does**:

The bridge always prefer another bridge as first node. If the bridge
only push/pull to 1 node per interval, then it will only communicate to
bridges, potentially leading to network partitioning if the gossiping is
not working to propagate changes.

To reduce the likelihood of network partitioning when gossiping is not
working and periodic push/pull is enabled, in this PR I configure the
bridge to push/pull to 2 nodes per interval (the first node is a bridge,
and the second node is selected randomly).

This change removes the need of enabling periodic rejoin from
`TestZoneAwareRouting_EndToEnd` because now we don't end up with network
partitioning when gossiping is disabled, and the state is propagated
exclusively using push/pull.

~~This PR is based on grafana/memberlist#11.
`go.mod` will be updated after memberlist PR will be merged.~~

**Which issue(s) this PR fixes**:

N/A

**Checklist**
- [x] Tests updated

---------

Signed-off-by: Marco Pracucci <marco@pracucci.com>
pracucci added a commit to grafana/mimir that referenced this pull request Nov 25, 2025
#### What this PR does
Update memberlist and dskit to grafana/dskit#798
and grafana/memberlist#11.

#### Which issue(s) this PR fixes or relates to

N/A

#### Checklist

- [ ] Tests updated.
- [ ] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Updates dskit and memberlist to add configurable multi-node push/pull;
bridges now push/pull with 2 nodes per interval (others with 1).
> 
> - **Memberlist (vendor fork)**:
> - Add `Config.PushPullNodes` with defaults set to `1` in
`DefaultLANConfig`, `DefaultWANConfig`, and `DefaultLocalConfig`.
> - Update `pushPull()` to select `N` random live nodes and sync with
each based on `PushPullNodes`.
> - **dskit (vendor)**:
> - In
`vendor/github.com/grafana/dskit/kv/memberlist/memberlist_client.go`,
set `mlCfg.PushPullNodes` to `2` for `NodeRoleBridge`, otherwise `1`.
> - **Dependencies**:
> - Bump `github.com/grafana/dskit` and replace
`github.com/hashicorp/memberlist` with newer
`github.com/grafana/memberlist` fork revisions.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
d2a441f. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Signed-off-by: Marco Pracucci <marco@pracucci.com>
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.

2 participants