Skip to content

Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. #29063

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 2 commits into from
Mar 15, 2018

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 14, 2018

I also had to make the test more lenient. This is due to the fact that
Lucene's RamUsageTester was changed in order not to reflect java.*
classes and the way that it estimates ram usage of maps is by assuming
it has similar memory usage to an Object[] array that stores all keys
and values. The implementation in LiveVersionMap tries to be slightly
more realistic by taking the load factor and linked lists into account,
so it usually gives a higher estimate which happens to be closer to
reality.

Closes #22548

I also had to make the test more lenient. This is due to the fact that
Lucene's RamUsageTester was changed in order not to reflect `java.*`
classes and the way that it estimates ram usage of maps is by assuming
it has similar memory usage to an `Object[]` array that stores all keys
and values. The implementation in `LiveVersionMap` tries to be slightly
more realistic by taking the load factor and linked lists into account,
so it usually gives a higher estimate which happens to be closer to
reality.

Closes elastic#22548
@jpountz jpountz requested a review from ywelsch March 14, 2018 14:36
@jpountz jpountz added the :Core/Infra/Core Core issues without another label label Mar 14, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jpountz jpountz added >test Issues or PRs that are addressing/adding tests v7.0.0 v6.3.0 labels Mar 14, 2018
@jpountz jpountz force-pushed the fix/live_version_map_ram_bytes_used branch from 407adff to a0b0f45 Compare March 14, 2018 14:42
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

This seems like an improvement to me, though I think someone with more knowledge of the test ought to take another look.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@jpountz jpountz merged commit 18d848f into elastic:master Mar 15, 2018
@jpountz jpountz deleted the fix/live_version_map_ram_bytes_used branch March 15, 2018 15:42
jpountz added a commit that referenced this pull request Mar 15, 2018
I also had to make the test more lenient. This is due to the fact that
Lucene's RamUsageTester was changed in order not to reflect `java.*`
classes and the way that it estimates ram usage of maps is by assuming
it has similar memory usage to an `Object[]` array that stores all keys
and values. The implementation in `LiveVersionMap` tries to be slightly
more realistic by taking the load factor and linked lists into account,
so it usually gives a higher estimate which happens to be closer to
reality.

Closes #22548
@jpountz
Copy link
Contributor Author

jpountz commented Mar 15, 2018

Thanks @nik9000 and @jasontedor!

martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/6.x: (89 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  [Docs] Fix link to Grok patterns (#29088)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  Fix Parsing Bug with Update By Query for Stored Scripts (#29039)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  [Docs] Fix Java Api index administration usage (#28260)
  Improve error message for installing plugin (#28298)
  Decouple XContentBuilder from BytesReference (#28972)
  ...
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/master: (97 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  Compilation fix for #29067
  [Docs] Fix link to Grok patterns (#29088)
  Store offsets in index prefix fields when stored in the parent field (#29067)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  CLI: Close subcommands in MultiCommand (#28954)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  Improve error message for installing plugin (#28298)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v6.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reenable LiveVersionMapTests.testRamBytesUsed on JDK9
5 participants