838055 - vcs-sync major alert for m-c branches

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[hwine]

Alerts from gd3 & gd2 starting at 1700 PT

Cause traced to bad time in recently added cset at: https://hg.mozilla.org/mozilla-central/rev/ffdee4a4eb7f

From within the conversion process:
    (Pdb) ctx.hex()
    'ffdee4a4eb7fc7cae80dfc4cb2fe0c3178773dcf'
    (Pdb) ctx.date()
    (1337962044.0, 25201)
Note that the 2nd element (timezone in seconds) is not divisible by 60.

This leads to dulwich error:
  File "/opt/vcs2vcs/venv/hg/lib/python2.6/site-packages/dulwich-0.8.5-py2.6-linux-x86_64.egg/dulwich/objects.py", line 1018, in format_timezone
    raise ValueError("Unable to handle non-minute offset.")

Comment 1

[hwine]

Workaround: to enable update of gecko.git for b2g, the following steps were taken:
 - disable update from m-c for a bit
 - strip the offending commit from the conversion repo
 - restart automated process

Comment 2

[hwine]

The values given above for time are directly from the patch that was landed today: see bug 819131 attachment 709252 [details] [diff] [review]

Note the unusual time: 
  # Date 1337962044 25201

Comment 3

[hwine]

Workaround: Also disabled ancient hg -> git conversion for github (w/o CVS history) running at:
  github-sync1-dev.dmz.scl3.mozilla.com:/opt/vcs2vcs/logs/job02

Comment 4

[(no longer active)]

I hit the same error (unsurprisingly) today as well, and here's how I fixed it:

diff --git a/hggit/git_handler.py b/hggit/git_handler.py
--- a/hggit/git_handler.py
+++ b/hggit/git_handler.py
@@ -361,16 +361,17 @@ class GitHandler(object):
         oldenc = self.swap_out_encoding()
 
         ctx = self.repo.changectx(rev)
         extra = ctx.extra()
 
         commit = Commit()
 
         (time, timezone) = ctx.date()
+        timezone = ((timezone / 60)*60)
         commit.author = self.get_git_author(ctx)
         commit.author_time = int(time)
         commit.author_timezone = -timezone
 
         if 'committer' in extra:
             # fixup timezone
             (name, timestamp, timezone) = extra['committer'].rsplit(' ', 2)
             commit.committer = name

This effecively rounds down the timezone offset to 25201.  Of course I don't think this is an appropriate fix at all, so I took out that hack right after I bypassed that error.

mccr8, which part of the world do you live in?  ;-)  But more seriously, do you know what you did out of the ordinary to end up with such a changeset?

Comment 5

[Justin Wood (:Callek)]

before hal called it a night, he theorized this was the leap-second manifesting in a cset and causing the bug.

Comment 6

[Andrew McCreight [:mccr8]]

I ended up with a weird time stamp because I manually edited the patch file.  I copied it over from another file and then decided to alter the time stamp (without knowing what it meant) because I was paranoid that having the same timestamp twice would be bad somehow.  I'm terribly sorry, I didn't realize that anything depended so critically on what the time stamp was. :(  "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots."

Comment 7

[hwine]

(In reply to Justin Wood (:Callek) from comment #5)
> before hal called it a night, he theorized this was the leap-second
> manifesting in a cset and causing the bug.

And that's why I slept on it ;) Although I did learn some there are some interesting differences between NTP time and POSIX time w.r.t. leap seconds.

Comment 8

[Andrew McCreight [:mccr8]]

FYI apparently there are a few time zones that are off by 15 or 30 minutes, which it looks like will get wiped out by the change in comment 4.

Comment 9

[hwine]

(In reply to Andrew McCreight [:mccr8] from comment #8)
> FYI apparently there are a few time zones that are off by 15 or 30 minutes,
> which it looks like will get wiped out by the change in comment 4.

Actually not - the value is in seconds, so 25200 = 7 hours, and the patch just rounds down the seconds to the next minute.

Comment 10

[Andrew McCreight [:mccr8]]

Ah, cool.  Thanks for the explanation.

Comment 11

[hwine]

Attachment: scrub input to protect dulwich

Our intent is to leave this patch in - it scrubs input before passing to dulwich.

Comment 12

[hwine]

Comment on attachment 710218 [details] [diff] [review]
scrub input to protect dulwich

committed:
 http://hg.mozilla.org/users/hwine_mozilla.com/hg-git/rev/9023fd488ada

created package hg_git-0.3.2-moz2.tar.gz and staged to http://scl3-production-puppet.srv.releng.scl3.mozilla.com/production/python-packages/

Comment 13

[(no longer active)]

Comment on attachment 710218 [details] [diff] [review]
scrub input to protect dulwich

This is clearly not the right thing to do, as pointed out in comment 8.  Please make sure that you revert this as soon as possible.  What I pointed out in comment 4 was a one time hack to get the conversion work going on.  Leaving this patch in will distort legitimate timezone information.

Comment 14

[hwine]

(In reply to :Ehsan Akhgari (Away 2/7-2/15) from comment #13)
> Comment on attachment 710218 [details] [diff] [review]
> scrub input to protect dulwich
> 
> This is clearly not the right thing to do, as pointed out in comment 8. 
> Please make sure that you revert this as soon as possible.  What I pointed
> out in comment 4 was a one time hack to get the conversion work going on. 
> Leaving this patch in will distort legitimate timezone information.

Please explain how it would distort legitimate timezone information. (Where legitimate is defined as "acceptable to dulwich code".

Comment 15

[(no longer active)]

(In reply to Hal Wine [:hwine] from comment #14)
> (In reply to :Ehsan Akhgari (Away 2/7-2/15) from comment #13)
> > Comment on attachment 710218 [details] [diff] [review]
> > scrub input to protect dulwich
> > 
> > This is clearly not the right thing to do, as pointed out in comment 8. 
> > Please make sure that you revert this as soon as possible.  What I pointed
> > out in comment 4 was a one time hack to get the conversion work going on. 
> > Leaving this patch in will distort legitimate timezone information.
> 
> Please explain how it would distort legitimate timezone information. (Where
> legitimate is defined as "acceptable to dulwich code".

That's the thing, the real bug is in dulwich, not in hg-git.  It's fine if you use this as a one-time hack to get the conversion script going again, but this is not the right fix here.

Comment 16

[(no longer active)]

Comment on attachment 710218 [details] [diff] [review]
scrub input to protect dulwich

I missed comment 9, sorry.

Comment 17

[(no longer active)]

Please make sure to upstream this fix to hg-git.  We should try not to diverge from hg-git if possible.

Thanks!

Comment 18

[hwine]

in production on all boxes, all workarounds from comment 1 and comment 3 undone.