zhaih commented on code in PR #13124:
URL: https://github.com/apache/lucene/pull/13124#discussion_r1522601711
##########
lucene/core/src/java/org/apache/lucene/index/MergeScheduler.java:
##########
@@ -52,6 +56,14 @@ public Directory wrapForMerge(OneMerge merge, Directory in) {
return in;
}
+ /**
+ * Provides an executor for parallelism during a single merge operation. By
default, the method
+ * returns `null` indicating that there is no parallelism during a single
merge operation.
Review Comment:
I think it's ok to force this method always return non-null? Since we
already have provided a good default?
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsFormat.java:
##########
@@ -152,7 +154,25 @@ public Lucene99HnswVectorsFormat() {
* @param beamWidth the size of the queue maintained during graph
construction.
*/
public Lucene99HnswVectorsFormat(int maxConn, int beamWidth) {
- this(maxConn, beamWidth, DEFAULT_NUM_MERGE_WORKER, null);
+ super("Lucene99HnswVectorsFormat");
+ if (maxConn <= 0 || maxConn > MAXIMUM_MAX_CONN) {
+ throw new IllegalArgumentException(
+ "maxConn must be positive and less than or equal to "
+ + MAXIMUM_MAX_CONN
+ + "; maxConn="
+ + maxConn);
+ }
+ if (beamWidth <= 0 || beamWidth > MAXIMUM_BEAM_WIDTH) {
+ throw new IllegalArgumentException(
+ "beamWidth must be positive and less than or equal to "
+ + MAXIMUM_BEAM_WIDTH
+ + "; beamWidth="
+ + beamWidth);
+ }
+ this.maxConn = maxConn;
+ this.beamWidth = beamWidth;
+ this.mergeExec = null;
+ this.numMergeWorkers = 1;
Review Comment:
Can remove those 2?
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsFormat.java:
##########
@@ -152,7 +154,25 @@ public Lucene99HnswVectorsFormat() {
* @param beamWidth the size of the queue maintained during graph
construction.
*/
public Lucene99HnswVectorsFormat(int maxConn, int beamWidth) {
- this(maxConn, beamWidth, DEFAULT_NUM_MERGE_WORKER, null);
+ super("Lucene99HnswVectorsFormat");
+ if (maxConn <= 0 || maxConn > MAXIMUM_MAX_CONN) {
+ throw new IllegalArgumentException(
+ "maxConn must be positive and less than or equal to "
+ + MAXIMUM_MAX_CONN
+ + "; maxConn="
+ + maxConn);
+ }
+ if (beamWidth <= 0 || beamWidth > MAXIMUM_BEAM_WIDTH) {
+ throw new IllegalArgumentException(
+ "beamWidth must be positive and less than or equal to "
+ + MAXIMUM_BEAM_WIDTH
+ + "; beamWidth="
+ + beamWidth);
+ }
+ this.maxConn = maxConn;
+ this.beamWidth = beamWidth;
+ this.mergeExec = null;
+ this.numMergeWorkers = 1;
Review Comment:
Oh I see, you haven't yet put in the change for auto figure out number of
workers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
