If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

sendchange sometimes hangs

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: catlee, Assigned: catlee)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
There have been a few cases over the past few days of sendchange steps never completing.  There's nothing in the logs to indicate why.  In at least one case, the remote buildbot master was blocked in FtpPoller, and restarting it allowed the local sendchange to complete.

We should add our own timeout handling to sendchange instead of relying on the twisted stack to time out.
I'm pretty sure you were looking into this Catlee. Throw it back if you weren't.
Assignee: nobody → catlee
(Assignee)

Comment 2

8 years ago
Created attachment 414271 [details] [diff] [review]
fun with twisted!

This adds a 60 second timeout to the sendchange operation, as well as making it interruptable.
Attachment #414271 - Flags: review?(bhearsum)
(Assignee)

Comment 3

8 years ago
Created attachment 414336 [details] [diff] [review]
fun with object oriented twisted!
Attachment #414271 - Attachment is obsolete: true
Attachment #414336 - Flags: review?(bhearsum)
Attachment #414271 - Flags: review?(bhearsum)
Attachment #414336 - Flags: review?(bhearsum) → review+
Comment on attachment 414336 [details] [diff] [review]
fun with object oriented twisted!

http://hg.mozilla.org/build/buildbotcustom/rev/18df53580c86
Attachment #414336 - Flags: checked-in+
pm01 got his change when it restarted at 2000 PST. pm02 is pending, I'll try to find a quiet time later tonight.
(Assignee)

Comment 6

8 years ago
Well, sendchanges don't hang any more...
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

8 years ago
Created attachment 416921 [details] [diff] [review]
Fix error handling
Attachment #416921 - Flags: review?(bhearsum)
Comment on attachment 416921 [details] [diff] [review]
Fix error handling

>diff --git a/steps/misc.py b/steps/misc.py
>--- a/steps/misc.py
>+++ b/steps/misc.py
>@@ -302,17 +302,18 @@ class SendChangeStep(BuildStep):
>     master: %s
>     branch: %s
>     revision: %s
>     comments: %s
>     user: %s
>     files: %s""" % (self.master, self.branch, self.revision, self.comments, self.user, self.files))
>             return self.sendChange()
>         except KeyError:
>-            return self.finished(Failure())
>+            self.addCompleteLog("errors", str(res))

I don't think 'res' is defined here. Maybe you want 'except KeyError, res', too?

>+            return self.finished(FAILURE)
> 
>     def sendChange(self):
>         d = self.sender.send(self.branch, self.revision, self.comments, self.files, self.user)
>         if self.timeout:
>             d = errbackAfter(d, self.timeout)
>         d = InterruptableDeferred(d)
>         self._interrupt = d.interrupt
>         d.addCallback(self.sendChangeSuccess)
>@@ -340,25 +341,25 @@ class SendChangeStep(BuildStep):
>                     return self.sendChangeFailed(reason)
>                 self._interrupt = interrupt
>                 return d
> 
>             self.step_status.setText(['sendchange to', self.master, 'failed'])
>             if self.warnOnFailure:
>                 self.step_status.setText2(['sendchange'])
>             self.addCompleteLog("errors", str(res))
>-            return BuildStep.finished(self, FAILURE)
>+            return self.finished(FAILURE)
>         except:
>             log.msg("Error processing sendchange failure")
>             log.err()
>-            return BuildStep.finished(self, FAILURE)
>+            return self.finished(FAILURE)
> 
>     def sendChangeSuccess(self, results):
>         self.step_status.setText(['sendchange to', self.master, 'ok'])
>-        return BuildStep.finished(self, SUCCESS)
>+        return self.finished(SUCCESS)
> 
>     def interrupt(self, reason):
>         self.retries = 0
>         if self._interrupt:
>             self._interrupt("Cancelled sendchange")
>             self._interrupt = None
> 
> class DownloadFile(ShellCommand):
Attachment #416921 - Flags: review?(bhearsum) → review-
(Assignee)

Comment 9

8 years ago
Created attachment 417499 [details] [diff] [review]
Fix error handling
Attachment #416921 - Attachment is obsolete: true
Attachment #417499 - Flags: review?(bhearsum)
Attachment #417499 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 10

8 years ago
Comment on attachment 417499 [details] [diff] [review]
Fix error handling

changeset:   550:2e4ffa62ff34
Attachment #417499 - Flags: checked-in+
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.