Skip to content

mgr/progress/module.py: s/events/_events/#29625

Merged
tchaikov merged 1 commit intoceph:masterfrom
kamoltat:wip-test-progress-fix-bug-#29385
Aug 20, 2019
Merged

mgr/progress/module.py: s/events/_events/#29625
tchaikov merged 1 commit intoceph:masterfrom
kamoltat:wip-test-progress-fix-bug-#29385

Conversation

@kamoltat
Copy link
Member

@kamoltat kamoltat commented Aug 13, 2019

Fixes time out failure because the module was trying to accessing
attribute events line 437 in this block of code:

435 if marked == "in":
436 for ev_id in list(self._events):
437 ev = self.events[ev_id]
438 if isinstance(ev, PgRecoveryEvent) and osd_id in ev.which_osds:
439 self.log.info("osd.{0} came back in, cancelling event".format(
440 osd_id
441 ))
442 self._complete(ev)

Therefore, it wouldn't trigger an event

Fixes: https://tracker.ceph.com/issues/40618

Signed-off-by: Kamoltat (Junior) Sirivadhna ksirivad@redhat.com

Checklist

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

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test docs
  • jenkins render docs

@tchaikov
Copy link
Contributor

@kamoltat thanks Junior, i think we can improve your commit message in two ways.

the title could be more concise: i'd suggest put something like

mgr/progress/module.py: s/events/_events/

might want to make the fix issue line more consistent with our convention, so it could be

Fixes: https://tracker.ceph.com/issues/40618

and, i don't quite understand how this change could address the failure of http://pulpito.ceph.com/kchai-2019-08-12_14:45:48-rados-wip-kefu-testing-2019-08-12-1306-distro-basic-mira/4211314/. even although it's correct and fix another issue.

as the failure is that

        self.wait_until_equal(lambda: len(self._events_in_progress()), 1,
                              timeout=self.EVENT_CREATION_PERIOD)

timed out after 5 seconds. not because of an AttributeError exception.

@kamoltat
Copy link
Member Author

@kamoltat thanks Junior, i think we can improve your commit message in two ways.

the title could be more concise: i'd suggest put something like

mgr/progress/module.py: s/events/_events/

might want to make the fix issue line more consistent with our convention, so it could be

Fixes: https://tracker.ceph.com/issues/40618

and, i don't quite understand how this change could address the failure of http://pulpito.ceph.com/kchai-2019-08-12_14:45:48-rados-wip-kefu-testing-2019-08-12-1306-distro-basic-mira/4211314/. even although it's correct and fix another issue.

as the failure is that

        self.wait_until_equal(lambda: len(self._events_in_progress()), 1,
                              timeout=self.EVENT_CREATION_PERIOD)

timed out after 5 seconds. not because of an AttributeError exception.

@tchaikov Hi Kefu, sorry for the bad commit message, will change it now. As for why correcting the attribute name leads to fixing, I tested this on my local machine by marking osd in and out and observing the event creation. So with the incorrect attributte name, the progress module still kept on going, only notify that it couldn't access the variable once. Therefore this leads to me thinking that fixing this attribute error would fix the problem but I could be wrong.

@kamoltat kamoltat changed the title mgr/progress & mgr/test_progress.py bug fix #29385 mgr/progress/module.py: s/events/_events/ Aug 13, 2019
@kamoltat
Copy link
Member Author

kamoltat commented Aug 13, 2019

@tchaikov Just to follow up on this, I think you are right that this is not a fix for Time out for 5 seconds. I had Neha queued a tautology test for me and it still fails, so will investigate further into this.

@kamoltat kamoltat force-pushed the wip-test-progress-fix-bug-#29385 branch from 9857457 to cfcd736 Compare August 14, 2019 18:15
@tchaikov
Copy link
Contributor

@kamoltat please note the commit message is different from the first comment in a PR. probably you could revise your commit using git commit --amend and repush to update your PR?

Fixes time out failure because the module was trying to accessing
attribute events line 437 in this block of code:

435 if marked == "in":
436 for ev_id in list(self._events):
437 ev = self.events[ev_id]
438 if isinstance(ev, PgRecoveryEvent) and osd_id in ev.which_osds:
439 self.log.info("osd.{0} came back in, cancelling event".format(
440 osd_id
441 ))
442 self._complete(ev)

Therefore, it wouldn't trigger an event

Fixes: https://tracker.ceph.com/issues/40618

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
@kamoltat kamoltat force-pushed the wip-test-progress-fix-bug-#29385 branch from cfcd736 to 297a0a3 Compare August 16, 2019 20:53
@tchaikov tchaikov merged commit 26d27a0 into ceph:master Aug 20, 2019
epuertat pushed a commit to rhcs-dashboard/ceph that referenced this pull request Aug 22, 2019
Fixes time out failure because the module was trying to accessing
attribute events line 437 in this block of code:

435 if marked == "in":
436 for ev_id in list(self._events):
437 ev = self.events[ev_id]
438 if isinstance(ev, PgRecoveryEvent) and osd_id in ev.which_osds:
439 self.log.info("osd.{0} came back in, cancelling event".format(
440 osd_id
441 ))
442 self._complete(ev)

Therefore, it wouldn't trigger an event

Fixes: https://tracker.ceph.com/issues/40618

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
mkogan1 pushed a commit to mkogan1/ceph that referenced this pull request Sep 5, 2019
Fixes time out failure because the module was trying to accessing
attribute events line 437 in this block of code:

435 if marked == "in":
436 for ev_id in list(self._events):
437 ev = self.events[ev_id]
438 if isinstance(ev, PgRecoveryEvent) and osd_id in ev.which_osds:
439 self.log.info("osd.{0} came back in, cancelling event".format(
440 osd_id
441 ))
442 self._complete(ev)

Therefore, it wouldn't trigger an event

Fixes: https://tracker.ceph.com/issues/40618

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
mkogan1 pushed a commit to mkogan1/ceph that referenced this pull request Sep 5, 2019
Fixes time out failure because the module was trying to accessing
attribute events line 437 in this block of code:

435 if marked == "in":
436 for ev_id in list(self._events):
437 ev = self.events[ev_id]
438 if isinstance(ev, PgRecoveryEvent) and osd_id in ev.which_osds:
439 self.log.info("osd.{0} came back in, cancelling event".format(
440 osd_id
441 ))
442 self._complete(ev)

Therefore, it wouldn't trigger an event

Fixes: https://tracker.ceph.com/issues/40618

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
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.

2 participants