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.

Sign up for GitHub

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

Jump to bottom

go.mod: update goavro from v2.8.1 to v2.9.8 #59745

Merged
merged 1 commit into from
Feb 9, 2021
Merged

go.mod: update goavro from v2.8.1 to v2.9.8 #59745

merged 1 commit into from
Feb 9, 2021

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Feb 3, 2021

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@pbardea pbardea mentioned this pull request Feb 4, 2021
Closed
6 tasks
@pbardea
Copy link
Contributor Author

pbardea commented Feb 4, 2021

Hm, interesting. A test is failing because of differing timestamps.

1684-10-05T17:51:40.795101Z -> 2269-04-26T17:26:14.504652Z
1690-05-19T13:29:46.145044Z -> 2274-12-08T13:04:19.854595Z
1706-02-05T02:38:08.15195Z -> 2290-08-26T02:12:41.861501Z

cc @HonoreDB , does anything jump out to you? This diff does have some changes with respect to timestamps in cockroachdb/vendored@26b10b3...44b3b8e#diff-550eddbb7dbace83d97dbfb9e67c78aa144f8182e29be6a890050319fc0b848f on line 157. Is it possible that the new version is actually what we want here?

@pbardea pbardea requested a review from HonoreDB February 4, 2021 16:19
@pbardea pbardea marked this pull request as ready for review February 4, 2021 16:19
@HonoreDB
Copy link
Contributor

HonoreDB commented Feb 4, 2021

Hm, interesting. A test is failing because of differing timestamps.

1684-10-05T17:51:40.795101Z -> 2269-04-26T17:26:14.504652Z
1690-05-19T13:29:46.145044Z -> 2274-12-08T13:04:19.854595Z
1706-02-05T02:38:08.15195Z -> 2290-08-26T02:12:41.861501Z

cc @HonoreDB , does anything jump out to you? This diff does have some changes with respect to timestamps in cockroachdb/vendored@26b10b3...44b3b8ediff-550eddbb7dbace83d97dbfb9e67c78aa144f8182e29be6a890050319fc0b848f on line 157. Is it possible that the new version is actually what we want here?

Yikes, yes. The timestamp actually created in the database for that first row is 2269-04-26 17:26:14.504652, so I'm guessing the assertions in the test are a result of someone copying the results of an integer overflow bug that is fixed in this update. Good thing you caught this before 2038!

@pbardea
Copy link
Contributor Author

pbardea commented Feb 4, 2021

Thanks for confirming! I'll update the tests along with the SDK bump.

@pbardea pbardea requested review from a team and miretskiy and removed request for a team February 4, 2021 22:17
HonoreDB
HonoreDB approved these changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@HonoreDB HonoreDB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @miretskiy)

@pbardea
go.mod: update goavro from v2.8.1 to v2.9.8
4fb2248 
Release note (bug fix): Fix a timestamp overflow bug that would overflow
timestamps after the year 2038 by updating the Avro SDK used by
changefeeds.
@pbardea
Copy link
Contributor Author

pbardea commented Feb 9, 2021

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Feb 9, 2021

Build succeeded:

@craig craig bot merged commit ef1ea06 into cockroachdb:master Feb 9, 2021
@HonoreDB HonoreDB mentioned this pull request Jul 13, 2021
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HonoreDB HonoreDB HonoreDB approved these changes

@miretskiy miretskiy Awaiting requested review from miretskiy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pbardea @cockroach-teamcity @HonoreDB