Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating from older version that have written time.Duration causes bad data - maybe add a note? #212

Closed
flaterik opened this issue Jul 10, 2018 · 5 comments

Comments

@flaterik
Copy link

We had been using an older version of the driver in production for some time, with many time.Duration values stored in the old nanos format. When we upgraded, all of those durations obviously came back 1e6 times larger than intended. Fixing the data was not difficult once we figured out what was wrong, and I do not foresee a code fix to this, but it might be nice to have a note about this change in the readme! It might save someone else the excitement that I had today.

@domodwyer
Copy link

Hi @flaterik

This is a little worrying - to my knowledge we haven't changed this serialisation - do you know which versions are the "old" and "new" in the above so we can put together a test?

Thanks,
Dom

@flaterik
Copy link
Author

flaterik commented Jul 16, 2018

Sorry, I definitely could have provided more info!
We moved from
package: gopkg.in/mgo.v2
version: 3f83fa5
(which I realize is quite old!)
to
package: github.com/globalsign/mgo
version: 113d396
This is the change that affected things -
go-mgo#373
And this is the code in question -

e.addInt64(int64(v.Int() / 1e6))

in = time.Duration(time.Duration(d.readInt64()) * time.Millisecond)

In the older incarnations that case wasn't there, so the durations were stored as the underlying time.Duration int64 as nanoseconds.
My solution was to scan our records and divide anything bigger than 1e15 by 1e6, which worked fine. Just had a period of time where things that were supposed to be five minutes were treated as if they should be 83,333.3333 hours.

@domodwyer
Copy link

Hi @flaterik

I had forgotten about this! I think it was pulled into my fork before it became a globalsign fork, and once it started becoming popular we decided to not make breaking changes to help users transition - this slipped through!

Thanks for the issue, we'll update the docs!

Dom

@aitva
Copy link

aitva commented Sep 20, 2018

Hi @domodwyer,

I am trying to migrate from gopkg.in/mgo.v2 and I have the same issue. Is there a recommended way to fix this? Can you point me at the doc?

Thanks!

@domodwyer
Copy link

Hi @aitva

Sorry about the hassle - we have tried pretty hard to maintain compatibility and this was the only slip!

@flaterik seems to have had good results with the strategy above:

My solution was to scan our records and divide anything bigger than 1e15 by 1e6, which worked fine.

But if that is difficult, an alternative is to include a conversion in the application but I would prefer the "change everything once" approach of @flaterik.

Dom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants