Skip to content

mgr/progress: fix dictionary change during iteration error#28840

Closed
kamoltat wants to merge 1 commit intoceph:masterfrom
kamoltat:wip-mgr-progress-fix-dictionary-change-iteration
Closed

mgr/progress: fix dictionary change during iteration error#28840
kamoltat wants to merge 1 commit intoceph:masterfrom
kamoltat:wip-mgr-progress-fix-dictionary-change-iteration

Conversation

@kamoltat
Copy link
Member

@kamoltat kamoltat commented Jul 2, 2019

Provide a fix for the bug where
there is runtime error for when
dictionary change during iteration.
Fix by just making a copy of the
dictionary using list()

Fixes: http://tracker.ceph.com/issues/40618
Signed-off-by: Kamoltat (Junior) Sirivadhna ksirivad@redhat.com

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Provide a fix for the bug where
there is runtime error for when
dictionary change during iteration.
Fix by just making a copy of the
dictionary using list()

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
@tchaikov tchaikov self-requested a review July 2, 2019 15:01
@neha-ojha
Copy link
Member

Hey @kamoltat, could you include the tracker reference for the bug that this PR is fixing in the commit message? See https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#tag-the-commit

@jdurgin
Copy link
Member

jdurgin commented Jul 2, 2019

if it's possible for a 2nd thread to change the dictionary, do we need to introduce a lock for accessing it instead?

Copy link
Member

@jdurgin jdurgin left a comment

Choose a reason for hiding this comment

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

I see what you mean now, about changing the dictionary during the loop. Looks good!

@tchaikov
Copy link
Contributor

http://pulpito.ceph.com/kchai-2019-07-09_13:03:00-rados-wip-kefu-testing-2019-07-09-1756-distro-basic-mira/4105666/

@kamoltat could you take a look? the tested branch contains this PR.

kamoltat added a commit to kamoltat/ceph that referenced this pull request Jul 30, 2019
Think the problem is how we are using dictionary.item() in
for loop causing runtime error therefore the event didn't
get created so we convert it to list(dictionary).

Fixes http://pulpito.ceph.com/kchai-2019-07-28_14:30:09-rados-wip-kefu2-testing-2019-07-28-1941-distro-basic-mira/4160881/

Also the same fix as: ceph#28840

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
kamoltat added a commit to kamoltat/ceph that referenced this pull request Aug 2, 2019
Think the problem is how we are using dictionary.item() in
for loop causing runtime error therefore the event didn't
get created so we convert it to list(dictionary).

Fixes http://pulpito.ceph.com/kchai-2019-07-28_14:30:09-rados-wip-kefu2-testing-2019-07-28-1941-distro-basic-mira/4160881/

Also the same fix as: ceph#28840

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
kamoltat added a commit to kamoltat/ceph that referenced this pull request Aug 5, 2019
Think the problem is how we are using dictionary.item() in
for loop causing runtime error therefore the event didn't
get created so we convert it to list(dictionary).

Fixes http://pulpito.ceph.com/kchai-2019-07-28_14:30:09-rados-wip-kefu2-testing-2019-07-28-1941-distro-basic-mira/4160881/

Also the same fix as: ceph#28840

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
kamoltat added a commit to kamoltat/ceph that referenced this pull request Aug 5, 2019
Think the problem is how we are using dictionary.item() in
for loop causing runtime error therefore the event didn't
get created so we convert it to list(dictionary).

Fixes http://pulpito.ceph.com/kchai-2019-07-28_14:30:09-rados-wip-kefu2-testing-2019-07-28-1941-distro-basic-mira/4160881/

Also the same fix as: ceph#28840

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
kamoltat added a commit to kamoltat/ceph that referenced this pull request Aug 5, 2019
Think the problem is how we are using dictionary.item() in
for loop causing runtime error therefore the event didn't
get created so we convert it to list(dictionary).

Fixes http://pulpito.ceph.com/kchai-2019-07-28_14:30:09-rados-wip-kefu2-testing-2019-07-28-1941-distro-basic-mira/4160881/

Also the same fix as: ceph#28840

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
@sebastian-philipp
Copy link
Contributor

any update on this?

@jdurgin
Copy link
Member

jdurgin commented Sep 13, 2019

superseded by #29035

@jdurgin jdurgin closed this Sep 13, 2019
@sebastian-philipp
Copy link
Contributor

also #29807

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants