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

Speed up doc tests using RamDirectory #51418

Closed
wants to merge 1 commit into from

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jan 24, 2020

Speeds up docs / x-pack rest tests by using a RamDirectory for writing out cluster state updates. These tests spin up a single node and do not require the state to be persisted.

Benchmarking on my CI machine for docs:check showed:

RAMDisk
master: 16:09 minutes
ram-dir-metadata: 14:49 minutes

SSD
master: 32:20 minutes
ram-dir-metadata: 16:32 minutes

Relates #49753

@mark-vieira
Copy link
Contributor

mark-vieira commented Jan 24, 2020

How does this explain the slowness when we run the build on a tmpfs volume in CI? In that scenario isn't everything written to ram? The graphs shown in this comment are builds exclusively using a ram-backed workspace directory. I wouldn't expect this change to help in that scenario.

Do we know what specific recent changes are associated with the additional setup work you mention here?

@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 24, 2020

See the benchmarking results above. This provides a smaller speed-up on RamDisk (as RamDirectory still seems to be faster to write to than FSDirectory), but helps quite a bit on regular disks, even SSDs.

This is an optional extension to #51430

@mark-vieira
Copy link
Contributor

I personally don't even like the fact that we are using ramdisks in CI. Users of ES do not generally run this way. It's simply a hack to squeeze more IO performance out of our build machines, but at the cost of an accurate testing environment. In reality, fsync is not a noop, but we are testing as though it is. Granted, we have separate performance testing but we should strive for our testing environments to match reality as much as possible. This seems like we are moving even further from that direction.

My preference would be for us not to introduce yet another hack which increases the delta between our test environment and a real production one just to squeeze more performance out of the build. The changes in #51430 already buys us a lot. The addition of this on top of that is much less dramatic and in a highly parallelized build the benefit would probably disappear entirely.

@@ -219,6 +222,9 @@ public static void deleteAll(Path[] dataPaths) throws IOException {

// exposed for tests
Directory createDirectory(Path path) throws IOException {
if (USE_RAM_DIR) {
return new RAMDirectory();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the new ByteBuffersDirectory which is the replacement for the deprecated RAMDirectory ?

@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 24, 2020

I'm fine omitting this hack if we deem #51430 good enough.

@ywelsch ywelsch closed this Jan 24, 2020
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