Skip to content

kvserver,rangefeed: ensure that iterators are only constructed under tasks#52844

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
ajwerner:ajwerner/construct-iterator-under-tasks
Aug 20, 2020
Merged

kvserver,rangefeed: ensure that iterators are only constructed under tasks#52844
craig[bot] merged 1 commit intocockroachdb:masterfrom
ajwerner:ajwerner/construct-iterator-under-tasks

Conversation

@ajwerner
Copy link
Copy Markdown
Contributor

Prior to this change, it was possible for a rangefeed request to be issued
concurrently with shutting down which could lead to an iterator being
constructed after the engine has been closed.

Touches #51544

Release note: None

@ajwerner ajwerner requested a review from andreimatei August 14, 2020 22:45
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/construct-iterator-under-tasks branch from ffa1cc4 to 571f4be Compare August 17, 2020 06:28
Copy link
Copy Markdown
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @andreimatei)


pkg/kv/kvserver/replica_rangefeed.go, line 196 at r1 (raw file):

	}

	// Register the stream with a catch-up iterator.

I think you want to move this comment inside the new function, right?

…tasks

Prior to this change, it was possible for a rangefeed request to be issued
concurrently with shutting down which could lead to an iterator being
constructed after the engine has been closed.

Touches cockroachdb#51544

Release note: None
Copy link
Copy Markdown
Contributor Author

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)


pkg/kv/kvserver/replica_rangefeed.go, line 196 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

I think you want to move this comment inside the new function, right?

meh, it's the existence of the function that indicates that the stream has been registered with a catch-up iterator.

@ajwerner ajwerner force-pushed the ajwerner/construct-iterator-under-tasks branch from 571f4be to 5cbf775 Compare August 19, 2020 12:24
@ajwerner
Copy link
Copy Markdown
Contributor Author

TFTR!

bors r=andreimatei

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 20, 2020

Build succeeded:

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