Skip to content

osd/scrub: ignoring unsolicited DigestUpdate events#44050

Merged
ronen-fr merged 1 commit intoceph:masterfrom
ronen-fr:wip-rf-digest-update
Mar 1, 2022
Merged

osd/scrub: ignoring unsolicited DigestUpdate events#44050
ronen-fr merged 1 commit intoceph:masterfrom
ronen-fr:wip-rf-digest-update

Conversation

@ronen-fr
Copy link
Contributor

@ronen-fr ronen-fr commented Nov 22, 2021

DigestUpdate events arriving too early, i.e. in WaitReplicas state, can
and should be ignored. The down-counter for pending digest updates is
checked upon entering state WaitDigestUpdates, and a zero counter will be
noticed then. And if the early event is a result of a bug -
handling it is a mistake.

Signed-off-by: Ronen Friedman rfriedma@redhat.com

@ronen-fr ronen-fr added the core label Nov 22, 2021
@ronen-fr ronen-fr marked this pull request as ready for review November 23, 2021 17:10
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Nov 24, 2021

http://pulpito.front.sepia.ceph.com/?branch=wip-rf-digest-update - 100 tests OK

@athanatos
Copy link
Contributor

athanatos commented Nov 29, 2021

Are we aware of a bug that causes this? I'm worried that this will hide problems with event ordering. Can we at least emit a central log warning and modify teuthology to fail any test that generates it?

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Dec 2, 2021

Are we aware of a bug that causes this? I'm worried that this will hide problems with event ordering. Can we at least emit a central log warning and modify teuthology to fail any test that generates it?

@athanatos :

  • there was such a bug, which caused those messages to be sent. The code that is removed here was my incorrect fix to that bug, many months ago: instead of analyzing why I'm seeing these events - I accommodated them... (the actual fix was part of previous PRs)

  • this PR is the opposite of hiding the bogus events. It is a result of a question you posed in another review ("how can these events be generated"). That was fixed, and now the code to hide them is removed.

I agree to your suggestions to send the messages to the cluster log, and fail relevant tests. Will modify accordingly.

@ronen-fr ronen-fr force-pushed the wip-rf-digest-update branch from 155383d to 63e2873 Compare December 2, 2021 13:07
@athanatos
Copy link
Contributor

athanatos commented Dec 2, 2021

I just want to make sure we actually notice if we start seeing these events again.

@github-actions
Copy link

github-actions bot commented Jan 24, 2022

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@ronen-fr ronen-fr force-pushed the wip-rf-digest-update branch from 63e2873 to fb2225e Compare February 17, 2022 09:31
@ronen-fr ronen-fr force-pushed the wip-rf-digest-update branch from fb2225e to 30a9486 Compare February 17, 2022 09:34
athanatos
athanatos reviewed Feb 17, 2022
View reviewed changes
@ronen-fr ronen-fr force-pushed the wip-rf-digest-update branch from 30a9486 to bf4d806 Compare February 24, 2022 11:03
@ronen-fr ronen-fr requested a review from neha-ojha February 24, 2022 14:20
@athanatos athanatos self-requested a review February 24, 2022 22:26
athanatos
athanatos approved these changes Feb 24, 2022
View reviewed changes
Copy link
Contributor

@athanatos athanatos left a comment

Choose a reason for hiding this comment

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

Seems ok to me.

@ronen-fr
osd/scrub: cluster-logging unsolicited DigestUpdate events
84fe362 
... and not just to the OSD's log

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
@ronen-fr ronen-fr force-pushed the wip-rf-digest-update branch from bf4d806 to 84fe362 Compare February 26, 2022 13:11
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Feb 27, 2022

I'll be merging this, based on Teuthology runs.

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Mar 1, 2022

jenkins test make check

@ronen-fr ronen-fr merged commit ded84b8 into ceph:master Mar 1, 2022
ronen-fr added a commit to ronen-fr/ceph that referenced this pull request Mar 23, 2022
@ronen-fr
osd/scrub: cluster-logging unsolicited DigestUpdate events
0eacac2 
... and not just to the OSD's log

Cherry-picking of PR ceph#44050. Manual editing was required
to fix the WaitDigestUpdate state reactions, as
the relevant change was missing in the picked commit
(was pushed by mistake by a separate PR)

(cherry picked from commit 84fe362)
Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
@ronen-fr ronen-fr mentioned this pull request Mar 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@athanatos athanatos athanatos approved these changes

@rzarzynski rzarzynski Awaiting requested review from rzarzynski

@amathuria amathuria Awaiting requested review from amathuria

@jdurgin jdurgin Awaiting requested review from jdurgin

@neha-ojha neha-ojha Awaiting requested review from neha-ojha

Assignees

No one assigned

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ronen-fr @athanatos