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

TmpFileIOPeons to create files under the merging output directory, instead of java.io.tmpdir #3990

Merged
merged 6 commits into from Mar 2, 2017

Conversation

leventov
Copy link
Member

@leventov leventov commented Mar 2, 2017

Made IndexMerger and IndexMergerV9 to create temporary files under the output directory + /tmpPeonFiles, instead of java.io.tmpdir.

This is needed to make it possible to point java.io.tmpdir to /tmp, and mount /tmp on tmpfs. Read the story about why keeping /tmp mounted on real disk is bad on boxes which run JVMs. https://issues.apache.org/jira/browse/CASSANDRA-9242 and https://groups.google.com/d/msg/mechanical-sympathy/9SP4IM-MUrI/mVZEK2cWDQAJ are also relevant.

Just mounting /tmp on tmpfs (without this change) makes TmpFileIOPeon pointless, because it exists to load some data off-memory.

…tput directory/tmpPeonFiles, instead of java.io.tmpdir
v8OutDir.mkdirs();
registerDeleteDirectory(closer, v8OutDir);
File tmpPeonFilesDir = new File(v8OutDir, "tmpPeonFiles");
tmpPeonFilesDir.mkdir();
Copy link
Contributor

Choose a reason for hiding this comment

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

This will pass even if the directory is not created. Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Used FileUtils.forceMkdir() here and in some other places in the codebase

@drcrallen
Copy link
Contributor

This changes a few ISE to IOException but that's probably more correct.

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

LGTM

registerDeleteDirectory(closer, v8OutDir);
File tmpPeonFilesDir = new File(v8OutDir, "tmpPeonFiles");
FileUtils.forceMkdir(tmpPeonFilesDir);
registerDeleteDirectory(closer, tmpPeonFilesDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

No reason to register a delete directory for this, it's contained within v8OutDir which is already registered.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's harmless and makes the code a little more "self-contained", if tmpPeonFilesDir if moved somewhere else (out of v8OutDir), so I would keep these calls

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough.


File tmpPeonFilesDir = new File(v9TmpDir, "tmpPeonFiles");
FileUtils.forceMkdir(tmpPeonFilesDir);
registerDeleteDirectory(closer, tmpPeonFilesDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

No reason to register deletion of this, it's contained within v9TmpDir.

@gianm gianm added this to the 0.10.1 milestone Mar 2, 2017
@gianm gianm merged commit 81a5f98 into apache:master Mar 2, 2017
@himanshug himanshug modified the milestones: 0.10.0, 0.10.1 Mar 2, 2017
@drcrallen drcrallen deleted the peon-temp-files-master branch March 2, 2017 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants