fix for ticket:2951: wait for the logs are created before finishing the step#1296
fix for ticket:2951: wait for the logs are created before finishing the step#1296sa2ajj merged 4 commits intobuildbot:masterfrom
Conversation
There is a race condition between addLogs calls, and the end of the step. for oldStyleSteps, this matters, because the subsequent addStderr, are only called at the end of the step, so we must wait for log are created before continuing Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
sa2ajj
changed the title
|
👍 Do you think we need a test case for this? |
|
Well, I do think so. I'd like to first have it tested by @benallard. The unit test will be a little bit hacky though, pretty unavoidable for testing race-conditions. |
tardyp
changed the title
|
added a testcase for this problem |
|
Fortunately, only |
|
So far, I was not able to get any exception any more in my status pages ! |
add a test for the case which the db is too slow to complete the newLog before end of the step
if I revert previous commit, I get the error reported by benallard
===============================================================================
[ERROR]
Traceback (most recent call last):
File "/Users/ptardy/dev/bb/buildbot-heroku/buildbot/master/buildbot/process/buildstep.py", line 157, in next
self._catchup()
File "/Users/ptardy/dev/bb/buildbot-heroku/buildbot/master/buildbot/process/buildstep.py", line 159, in _catchup
self.step._start_unhandled_deferreds.append(d)
exceptions.AttributeError: 'NoneType' object has no attribute 'append'
buildbot.test.integration.test_custom_buildstep.RunSteps.test_OldStyleCustomBuildStepSlowDB
buildbot.test.integration.test_custom_buildstep.RunSteps.test_OldStyleCustomBuildStepSlowDB
===============================================================================
[ERROR]
Traceback (most recent call last):
File "/Users/ptardy/dev/bb/buildbot-heroku/buildbot/master/buildbot/process/buildstep.py", line 135, in gotAsync
self._catchup()
File "/Users/ptardy/dev/bb/buildbot-heroku/buildbot/master/buildbot/process/buildstep.py", line 159, in _catchup
self.step._start_unhandled_deferreds.append(d)
exceptions.AttributeError: 'NoneType' object has no attribute 'append'
buildbot.test.integration.test_custom_buildstep.RunSteps.test_OldStyleCustomBuildStepSlowDB
Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
|
yeah!! just fixed the silly import errors.. |
|
I do not find them to be silly: less imports less maintenance burden :) |
There was a problem hiding this comment.
I love the fix, but this has me worried because it's using clock time to determine which side of a race wins. Could you add a hook at the beginning of finally block run run, and use that hook to call reactor.callLater(0, delayed) in the tests? That would mean that newLog doesn't return until the reactor iteration after the yield statement.
There was a problem hiding this comment.
could you be more specific, on how you would write this hook. I can't think of any solution that wouldn't be horribly intrusive.
There was a problem hiding this comment.
If that's OK by you, cherry-pick it into this pull req, or let me know and I'll merge both.
Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
|
includes @djmitche 's hook. I improved it to use assert, so that it is optimized out in prod. |
fix for ticket:2951: wait for the logs are created before finishing the step
|
Thanks ! |
5 participants
There is a race condition between addLogs calls, and the end of the step.
for oldStyleSteps, this matters, because the subsequent addStderr, are only called
at the end of the step, so we must wait for log are created before continuing
Signed-off-by: Pierre Tardy pierre.tardy@intel.com