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

importccl: remove unnecessary interface function #41972

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

spaskob
Copy link
Contributor

@spaskob spaskob commented Oct 28, 2019

importccl: remove unnecessary interface function

This is part of an ongoing refactor to simplify the IMPORT code base.
Particularly here we remove calls to inputFinished which is supposed to be
called after all input files are ingested to close the cannel kvCh on which
the KVs are sent to the routine that drains this channel and sends them to KV.
Instead the creation and close of the channel is moved closer to where it is used.
inputFinished was really only used for a special case in CSV where we have a fan out to
a set of workers that forward the KVs to kvCh and so the closing logic needs to be called
after these workers are done. Now instead the reading of the files and the workers are grouped
so that we can wait for all routines from the group to finish and then close the channel.
This will simplify how we save rejected rows. See the issue below.

Touches: #40374.

Release note: none.

@spaskob spaskob requested a review from thoszhang October 28, 2019 21:41
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@spaskob spaskob force-pushed the refactor branch 2 times, most recently from ed153fd to 53c198b Compare October 29, 2019 22:00
This is part of an ongoing refactor to simplify the IMPORT code base.
Particularly here we remove calls to inputFinished which is supposed to be
called after all input files are ingested to close the cannel kvCh on which
the KVs are sent to the routine that drains this channel and sends them to KV.
Instead the creation and close of the channel is moved closer to where it is used.
inputFinished was really only used for a special case in CSV where we have a fan out to
a set of workers that forward the KVs to kvCh and so the closing logic needs to be called
after these workers are done. Now instead the reading of the files and the workers are grouped
so that we can wait for all routines from the group to finish and then close the channel.
This will simplify how we save rejected rows. See the issue below.

Touches: cockroachdb#40374.

Release note: none.
Copy link
Contributor

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 10 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@thoszhang
Copy link
Contributor

Also, could you paste the new commit message into the PR description?

@spaskob
Copy link
Contributor Author

spaskob commented Oct 30, 2019

bors r+

craig bot pushed a commit that referenced this pull request Oct 30, 2019
41972: importccl: remove unnecessary interface function r=spaskob a=spaskob

importccl: remove unnecessary interface function
    
This is part of an ongoing refactor to simplify the IMPORT code base.
 Particularly here we remove calls to inputFinished which is supposed to be
 called after all input files are ingested to close the cannel kvCh on which
 the KVs are sent to the routine that drains this channel and sends them to KV.
 Instead the creation and close of the channel is moved closer to where it is used.
 inputFinished was really only used for a special case in CSV where we have a fan out to
 a set of workers that forward the KVs to kvCh and so the closing logic needs to be called
 after these workers are done. Now instead the reading of the files and the workers are grouped
 so that we can wait for all routines from the group to finish and then close the channel.
 This will simplify how we save rejected rows. See the issue below.
 
 Touches: #40374.
    
 Release note: none.

Co-authored-by: Spas Bojanov <[email protected]>
@craig
Copy link
Contributor

craig bot commented Oct 30, 2019

Build succeeded

@craig craig bot merged commit 736e88d into cockroachdb:master Oct 30, 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