Small enhancements to IndexWriter's InfoStream to support segment tracing #14837
Conversation
when working on the new segment tracing tool from luceneutil (example: https://githubsearch.mikemccandless.com/segments_15.html): * KNN merging was logging "start/done" multiple times and then I realized it was for a different field each time, so I added field name * Thread name was duplicated in a single line at end of full flush * Defensive change: don't invoke merge-on-commit logic if merge policy returns empty list of merges * I enhanced merge-on-commit logging to state how many merges it will do, and whether all merges completed in the timeout window or not * TMP now also logs its deletesPctAllowed and forceMerteDeletesPctAllowed, and more details when it kicks off merge selection
github-project-automation
bot
moved this to Open
in OpenSearch Lucene & Core Performance Tracking
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
|
Seems a bit weird to include a merge policy change in a PR that is mostly about changing logging, but OK. I think I would be happier if the PR was entitled "Don't invoke merge-on-commit logic if merge policy returns empty list of merges" and secondarily patched infoStream logging. |
| @@ -3724,18 +3724,25 @@ private long prepareCommitInternal() throws IOException { | |||
| maybeCloseOnTragicEvent(); | |||
| } | |||
|
|
|||
| if (pointInTimeMerges != null) { | |||
| if (pointInTimeMerges != null && pointInTimeMerges.merges.size() > 0) { | |||
| if (infoStream.isEnabled("IW")) { | |||
| infoStream.message( | |||
| "IW", "now run merges during commit: " + pointInTimeMerges.segString(directory)); | |||
There was a problem hiding this comment.
There's a TODO to deprecate segString(Directory dir), and indeed the directory is not used anywhere. We can simply use toString() instead. I just happened to find it as I wanted to make sure we log the number of submitted merges. We don't need to address it in this PR, in fact, it could be a good first issue.
There was a problem hiding this comment.
created #14899 as a "good first issue"
There was a problem hiding this comment.
Thanks @vigyasharma -- good idea!
Oh I can revert that part if you want? And break it into its own PR ... My primary purpose here was to fix confusing issues with the |
mikemccand
added
the
skip-changelog
|
OK I restored that |
Some small fixes to IndexWriter's InfoStream logging, uncovered when working on the new segment tracing tool from luceneutil (example: https://githubsearch.mikemccandless.com/segments_15.html). I've been sitting on these changes in my main dev area and finally got around to making a PR before I lose track of them, phew!
Details: