Skip to content

Use JNA to Speed up Snapshot Cache File Creation #68687

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

Merged
merged 14 commits into from
Feb 10, 2021

Conversation

original-brownbear
Copy link
Contributor

@original-brownbear original-brownbear commented Feb 8, 2021

Use JNA to speed up snapshot cache file creation. Do this in :server
to bypass the security filter and move necessary bits of code to :server
to enable the logic.
Fall back to trying to create the file by writing zeros if anything except for
the step determining free disk space fails.

Manually verified via du that this works as expected and actually allocates hundreds of GB in a few ms.

Use JNA to speed up snapshot cache file creation. Do this in `:server`
to bypass the security filter and move necessary bits of code to `:server`
to enable the logic.
Fall back to trying to create the file by writing zeros if anything except for
the step determining free disk space fails.
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 8, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@@ -260,4 +269,39 @@ static void tryInstallSystemCallFilter(Path tmpFile) {
logger.warn("unable to install syscall filter: ", e);
}
}

@SuppressForbidden(reason = "need access to fd on FileOutputStream")
static void fallocateSnapshotCacheFile(Environment environment, long fileSize) throws IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little unfortunate to leak that much logic into this class, but I figured abstracting things away further to isolate the fallocate as much as possible wasn't really worth the extra complication (IMO).

@original-brownbear
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM. Would like to see a review from one more person before merge

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM

*/
public static void tryCreateCacheFile(Environment environment, long fileSize) throws IOException {
if (JNA_AVAILABLE == false) {
logger.warn("cannot use fallocate to create cache file because JNA is not available");
Copy link
Member

Choose a reason for hiding this comment

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

nit: I know we are not consistent in starting log messages with uppercase or not but we're using both ways in this PR

@original-brownbear
Copy link
Contributor Author

Thanks Yannick + Tanguy!

@original-brownbear original-brownbear merged commit 3b249fa into elastic:master Feb 10, 2021
@original-brownbear original-brownbear deleted the jna-magic branch February 10, 2021 13:26
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Feb 10, 2021
Use JNA to speed up snapshot cache file creation. Do this in `:server`
to bypass the security filter and move necessary bits of code to `:server`
to enable the logic.
Fall back to trying to create the file by writing zeros if anything except for
the step determining free disk space fails.
original-brownbear added a commit that referenced this pull request Feb 10, 2021
Use JNA to speed up snapshot cache file creation. Do this in `:server`
to bypass the security filter and move necessary bits of code to `:server`
to enable the logic.
Fall back to trying to create the file by writing zeros if anything except for
the step determining free disk space fails.
joegallo added a commit that referenced this pull request Feb 10, 2021
joegallo added a commit that referenced this pull request Feb 10, 2021
@joegallo
Copy link
Contributor

Reverted in 4c2e9a6 because it broke local development on a mac. (Sorry!)

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Feb 10, 2021
Second go at elastic#68687 now using the proper indirection so that this works
on both OSX and Linux. We can't use the same class we use for `mlock` like the original PR did
because the method signature differs across OSX and Linux.
original-brownbear added a commit that referenced this pull request Feb 10, 2021
Second go at #68687 now using the proper indirection so that this works
on both OSX and Linux. We can't use the same class we use for `mlock` like the original PR did
because the method signature differs across OSX and Linux.
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Feb 11, 2021
Second go at elastic#68687 now using the proper indirection so that this works
on both OSX and Linux. We can't use the same class we use for `mlock` like the original PR did
because the method signature differs across OSX and Linux.
original-brownbear added a commit that referenced this pull request Feb 11, 2021
Second go at #68687 now using the proper indirection so that this works
on both OSX and Linux. We can't use the same class we use for `mlock` like the original PR did
because the method signature differs across OSX and Linux.
@original-brownbear original-brownbear restored the jna-magic branch April 18, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.12.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants