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

row: move cfetcher initialization validation steps outside of loop #43009

Merged
merged 1 commit into from
Dec 6, 2019

Conversation

rohany
Copy link
Contributor

@rohany rohany commented Dec 5, 2019

Some initialization checks that the cfetcher performed were occuring
in a loop for no reason. This PR moves them out of the loop.

Release note: None

@rohany rohany requested review from jordanlewis and a team December 5, 2019 23:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

:lgtm: Good 👀

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

Some initialization checks that the cfetcher performed were occuring
in a loop for no reason. This PR moves them out of the loop.

Release note: None
@rohany rohany force-pushed the cfetcher-init-fix branch from ec54220 to a6b779c Compare December 5, 2019 23:37
@rohany
Copy link
Contributor Author

rohany commented Dec 6, 2019

bors r=yuzefovich

craig bot pushed a commit that referenced this pull request Dec 6, 2019
42952: pgdate: fix parsing of dates before unix epoch r=otan a=otan

Resolves #42937.

Previously, pgdate handling dates before the unix epoch with time
attached to it rounded up instead of rounding down, meaning times such
as '1969-12-30 01:00:00' rounded up to days = `-1`, when it should be
days = `-2`. This PR addresses that change.

I will probably look at backporting this.

Release note (bug fix): We did not previously handled date casts from
timestamp/timestamptz with time attached to it for times before the unix
epoch correctly. For example, '1969-12-30 01:00:00'::timestamp would
round to '1969-12-31' instead of '1969-12-30'. This PR addresses that
change.

43006: ccl/changefeedccl: respect filesystem walk errors in cloudFeed r=nvanbenschoten a=nvanbenschoten

Closes #42979.

This commit properly handles errors in `cloudFeed`'s `filepath.WalkFunc`
implementation. The documentation says:
> If there was a problem walking to the file or directory named by
> path, the incoming error will describe the problem and the function
> can decide how to handle that error (and Walk will not descend into
> that directory). If an error is returned, processing stops.

I stressed the test for 10,000 iterations and never saw anything, so it's
possible that this was a fluke. It's not clear what filesystem error this was
throwing so we might still see this pop up again, but at least we'll now
correctly propagate the error and surface it instead of hitting an NPE.
Because of that, I'm closing the issue for now.

Release note: None

43009: row: move cfetcher initialization validation steps outside of loop r=yuzefovich a=rohany

Some initialization checks that the cfetcher performed were occuring
in a loop for no reason. This PR moves them out of the loop.

Release note: None

43010: cmd/roachtest: only log sqlsmith on non-errors r=mjibson a=mjibson

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Rohan Yadav <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
@craig
Copy link
Contributor

craig bot commented Dec 6, 2019

Build succeeded

@craig craig bot merged commit a6b779c into cockroachdb:master Dec 6, 2019
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

Successfully merging this pull request may close these issues.

3 participants