Skip to content

Split notifier select in 2 to ensure newer targets are used.#10948

Merged
roidelapluie merged 3 commits intoprometheus:mainfrom
roidelapluie:notifierselect
Jul 1, 2022
Merged

Split notifier select in 2 to ensure newer targets are used.#10948
roidelapluie merged 3 commits intoprometheus:mainfrom
roidelapluie:notifierselect

Conversation

@roidelapluie
Copy link
Copy Markdown
Member

Fix #8768

Signed-off-by: Julien Pivotto [email protected]

Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@roidelapluie
Copy link
Copy Markdown
Member Author

My last push was only gofumpt, so I'll go ahead and auto merge this.

@roidelapluie roidelapluie enabled auto-merge (squash) July 1, 2022 10:14
@roidelapluie roidelapluie disabled auto-merge July 1, 2022 10:28
@roidelapluie
Copy link
Copy Markdown
Member Author

@SuperQ can you review again? I removed the flakiness by running the test 10 times with timeoutes 10 limes lower (so it is still pretty fast).

Signed-off-by: Julien Pivotto <[email protected]>
Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Yup, works for me.

@roidelapluie roidelapluie merged commit 02f3297 into prometheus:main Jul 1, 2022
machine424 added a commit to machine424/prometheus that referenced this pull request Jun 3, 2024
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.

The existing `TestHangingNotifier` that was added in prometheus#10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.

The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.

Signed-off-by: machine424 <[email protected]>

Co-authored-by: Ethan Hunter <[email protected]>

test: use atomic to prevent races.
machine424 added a commit to machine424/prometheus that referenced this pull request Jun 3, 2024
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.

The existing `TestHangingNotifier` that was added in prometheus#10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.

The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.

Signed-off-by: machine424 <[email protected]>

Co-authored-by: Ethan Hunter <[email protected]>
machine424 added a commit to machine424/prometheus that referenced this pull request Jun 10, 2024
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.

The existing `TestHangingNotifier` that was added in prometheus#10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.

The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.

Signed-off-by: machine424 <[email protected]>

Co-authored-by: Ethan Hunter <[email protected]>
machine424 added a commit to machine424/prometheus that referenced this pull request Jun 19, 2024
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.

The existing `TestHangingNotifier` that was added in prometheus#10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.

The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.

Signed-off-by: machine424 <[email protected]>

Co-authored-by: Ethan Hunter <[email protected]>
machine424 added a commit that referenced this pull request Jun 19, 2024
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.

The existing `TestHangingNotifier` that was added in #10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.

The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.

Signed-off-by: machine424 <[email protected]>

Co-authored-by: Ethan Hunter <[email protected]>
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.

DNS discovery fails to sync if AlertManager has connection timeout

2 participants