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
LUCENE-9662: Update concurrent index checking usage instructions #281
LUCENE-9662: Update concurrent index checking usage instructions #281
Conversation
…mpression (apache#281) Co-authored-by: Matthew Sporleder <matt.sporleder@multiply.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zacharymorn -- I left a couple small comments.
@@ -3994,6 +3994,9 @@ public static Options parseOptions(String[] args) { | |||
+ " -segment X: only check the specified segments. This can be specified multiple\n" | |||
+ " times, to check more than one segment, eg '-segment _2 -segment _a'.\n" | |||
+ " You can't use this with the -exorcise option\n" | |||
+ " -threadCount X: number of new threads created and used to check index concurrently.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just say number of threads used to check index concurrently
? I.e. drop the "new" because then I start to wonder if main
thread counts :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes sorry :D . Updated the instruction.
@@ -3994,6 +3994,9 @@ public static Options parseOptions(String[] args) { | |||
+ " -segment X: only check the specified segments. This can be specified multiple\n" | |||
+ " times, to check more than one segment, eg '-segment _2 -segment _a'.\n" | |||
+ " You can't use this with the -exorcise option\n" | |||
+ " -threadCount X: number of new threads created and used to check index concurrently.\n" | |||
+ " When not specified, this will default to the number of CPU cores up to 4.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should change this default to not cap at 4
? Just use number of cores? This is command-line execution, which is typically done only once at a time (versus when CheckIndex
is invoked from our tests...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Updated the code and instruction here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks @zacharymorn!
Thanks Michael for the review and approval! |
Description
Update concurrent index checking usage instructions
Sample output
Checklist
Please review the following and check all that apply:
main
branch../gradlew check
.