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

distsqlrun: add missing close call to backfiller #36604

Merged
merged 1 commit into from
Apr 8, 2019
Merged

Conversation

dt
Copy link
Member

@dt dt commented Apr 8, 2019

Release note: none.

@dt dt requested review from thoszhang, vivekmenezes and a team April 8, 2019 02:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@vivekmenezes vivekmenezes left a comment

Choose a reason for hiding this comment

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

Seems okay, but then why have this in the first place if it's not fixing something?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lucy-zhang and @vivekmenezes)

@dt
Copy link
Member Author

dt commented Apr 8, 2019

I'm not sure I follow: it is fixing something -- the missing close() that the interface stipulates must be called if prepare is called.

In this case that a) prints the on-close summary to the log and b) avoid leaking memory in the c++ SST builder on early error returns.

@vivekmenezes
Copy link
Contributor

Thanks. LGTM!

@dt
Copy link
Member Author

dt commented Apr 8, 2019

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 8, 2019

Build failed

@dt
Copy link
Member Author

dt commented Apr 8, 2019

🥐
bors r+

craig bot pushed a commit that referenced this pull request Apr 8, 2019
36604: distsqlrun: add missing close call to backfiller r=dt a=dt

Release note: none.

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

craig bot commented Apr 8, 2019

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.

4 participants