dsmiley commented on code in PR #4064:
URL: https://github.com/apache/solr/pull/4064#discussion_r2724276176


##########
changelog/unreleased/SOLR-7003.yml:
##########
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Refactor passing commit options through SolrClient. Remove unused 
waitFlush parameter.
+type: changed # added, changed, fixed, deprecated, removed, dependency_update, 
security, other

Review Comment:
   I hate the name of this enum.  "changed" is so generic that it would seem to 
apply to anything.  "other" is for refactoring.
   CC @janhoy seen this issue multiple times now



##########
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java:
##########
@@ -1084,7 +1084,7 @@ protected long waitToSeeSampleDocs(String collectionName, 
long numAdded)
       // wait up to 5 seconds for this to occur
       final long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
       do {
-        cloudSolrClient.commit(collectionName, true, true, true);
+        cloudSolrClient.commit(collectionName, 
CommitOptions.forSoftCommit().waitSearcher(true));

Review Comment:
   waitSearcher should be the default



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the waitSearcher value changed.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance with updated waitSearcher
+   */
+  public CommitOptions withWaitSearcher(boolean waitSearcher) {
+    return waitSearcher(waitSearcher);
+  }
+
+  /**
+   * Sets whether to open a new searcher as part of the commit. Returns a new 
CommitOptions instance
+   * with the updated value.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions openSearcher(boolean openSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the openSearcher value changed.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance with updated openSearcher
+   */
+  public CommitOptions withOpenSearcher(boolean openSearcher) {

Review Comment:
   pointless



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the waitSearcher value changed.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance with updated waitSearcher
+   */
+  public CommitOptions withWaitSearcher(boolean waitSearcher) {
+    return waitSearcher(waitSearcher);
+  }
+
+  /**
+   * Sets whether to open a new searcher as part of the commit. Returns a new 
CommitOptions instance
+   * with the updated value.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions openSearcher(boolean openSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the openSearcher value changed.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance with updated openSearcher
+   */
+  public CommitOptions withOpenSearcher(boolean openSearcher) {
+    return openSearcher(openSearcher);
+  }
+
+  /**
+   * Returns a new CommitOptions with the softCommit value changed.
+   *
+   * @param softCommit true for soft commit, false for hard commit
+   * @return new CommitOptions instance with updated softCommit
+   */
+  public CommitOptions withSoftCommit(boolean softCommit) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Sets whether to expunge deleted documents during optimize. Returns a new 
CommitOptions instance

Review Comment:
   false.  expunge deletes is exclusive with "optimize".
   
https://github.com/dsmiley/solr/blob/6d52d5fbf2fd46ef7e6333a956f105a29adfaa24/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java#L865



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the waitSearcher value changed.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance with updated waitSearcher
+   */
+  public CommitOptions withWaitSearcher(boolean waitSearcher) {
+    return waitSearcher(waitSearcher);
+  }
+
+  /**
+   * Sets whether to open a new searcher as part of the commit. Returns a new 
CommitOptions instance
+   * with the updated value.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions openSearcher(boolean openSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the openSearcher value changed.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance with updated openSearcher
+   */
+  public CommitOptions withOpenSearcher(boolean openSearcher) {
+    return openSearcher(openSearcher);
+  }
+
+  /**
+   * Returns a new CommitOptions with the softCommit value changed.
+   *
+   * @param softCommit true for soft commit, false for hard commit
+   * @return new CommitOptions instance with updated softCommit
+   */
+  public CommitOptions withSoftCommit(boolean softCommit) {

Review Comment:
   again, terrible docs.



##########
solr/benchmark/src/java/org/apache/solr/bench/MiniClusterState.java:
##########
@@ -501,7 +502,9 @@ public void forceMerge(String collection, int 
maxMergeSegments) throws Exception
 
         UpdateRequest optimizeRequest = new UpdateRequest();
         final var url = 
nodes.get(random.nextInt(cluster.getJettySolrRunners().size()));
-        optimizeRequest.setAction(UpdateRequest.ACTION.OPTIMIZE, false, true, 
maxMergeSegments);
+        optimizeRequest.setAction(
+            UpdateRequest.ACTION.OPTIMIZE,
+            CommitOptions.forOptimize(maxMergeSegments).waitSearcher(true));

Review Comment:
   waitSearcher ought to be the default; is it not?



##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##########
@@ -247,7 +248,8 @@ private static UpdateResponse softCommit(
       HttpJettySolrClient solrClient, String baseUrl, String coreName)
       throws SolrServerException, IOException {
     UpdateRequest ureq = new UpdateRequest();
-    ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, false, true, true);
+    ureq.setAction(
+        AbstractUpdateRequest.ACTION.COMMIT, 
CommitOptions.forSoftCommit().waitSearcher(true));

Review Comment:
   waitSearcher ought to be the default



##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -302,8 +302,10 @@ private void commitOnLeader(String leaderBaseUrl, String 
coreName)
       // ureq.getParams().set(DistributedUpdateProcessor.COMMIT_END_POINT, 
true);
       // ureq.getParams().set(UpdateParams.OPEN_SEARCHER, onlyLeaderIndexes);
       // Why do we need to open searcher if "onlyLeaderIndexes"?
-      ureq.getParams().set(UpdateParams.OPEN_SEARCHER, false);
-      ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, false, 
true).process(client);
+      ureq.setAction(
+              AbstractUpdateRequest.ACTION.COMMIT,
+              
CommitOptions.forHardCommit().openSearcher(false).waitSearcher(true))

Review Comment:
   we're not going to open a searcher, and yet we will wait for it ?!



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {

Review Comment:
   as this relates to optimize (only), I don't think it goes here.  It doesn't 
have to do with the commit, even though optimize accompanies a commit.



##########
solr/core/src/test/org/apache/solr/cli/ApiToolTest.java:
##########
@@ -66,7 +67,8 @@ public void testQueryResponse() throws Exception {
     cluster.waitForActiveCollection(COLLECTION_NAME, 2, 2);
 
     UpdateRequest ur = new UpdateRequest();
-    ur.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true);
+    ur.setAction(
+        AbstractUpdateRequest.ACTION.COMMIT, 
CommitOptions.forHardCommit().waitSearcher(true));

Review Comment:
   waitSearcher is the default.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {

Review Comment:
   For boolean options, I think we only need a method that changes the default. 
 Thus `dontWaitForSearcher`.
   
   But moreover, I think this issue allows us to reconsider our questionable 
naming of the past.  We're exposing internal implementation details in this 
name.  Users should not need to know what a "searcher" even is!  The simple 
explanation of this option, is that it toggles whether this commit is 
asynchronous or not.  By default, commits are synchronous -- we wait for the 
full effect.  WDYT?
   
   Also, despite this documentation that an LLM probably wrote, it's just 
"slop" frankly.  We have not helped the user understand what these options even 
mean!  Probably needs to be written by a human.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,

Review Comment:
   expungeDeletes is more of a command (like optimize), that just so happens to 
accompany a commit (like optimize).  It is potentially expensive, not as much 
as optimize generally but still.  I don't think it goes here.



##########
solr/core/src/test/org/apache/solr/AnalysisAfterCoreReloadTest.java:
##########
@@ -57,7 +58,7 @@ public void testStopwordsAfterCoreReload() throws Exception {
     // default stopwords - stopworda and stopwordb
 
     UpdateRequest up = new UpdateRequest();
-    up.setAction(ACTION.COMMIT, true, true);
+    up.setAction(ACTION.COMMIT, 
CommitOptions.forHardCommit().waitSearcher(true));

Review Comment:
   waitSearcher is the default



##########
solr/core/src/test/org/apache/solr/cli/TestExportTool.java:
##########
@@ -90,7 +91,8 @@ public void testBasic() throws Exception {
       Path baseDir = cluster.getBaseDir();
 
       UpdateRequest ur = new UpdateRequest();
-      ur.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true);
+      ur.setAction(
+          AbstractUpdateRequest.ACTION.COMMIT, 
CommitOptions.forHardCommit().waitSearcher(true));

Review Comment:
   waitSearcher is the default.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the waitSearcher value changed.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance with updated waitSearcher
+   */
+  public CommitOptions withWaitSearcher(boolean waitSearcher) {
+    return waitSearcher(waitSearcher);
+  }
+
+  /**
+   * Sets whether to open a new searcher as part of the commit. Returns a new 
CommitOptions instance
+   * with the updated value.
+   *
+   * @param openSearcher true to open a new searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions openSearcher(boolean openSearcher) {

Review Comment:
   Again, a boolean option... and one that exposes internal Solr details -- 
searcher.  IMO we should have this method be called `flushOnly()` as it conveys 
the effect of setting this to false.
   
   I suppose fellow Solr/Lucene geeks could debate a true flush only from a 
commit that doesn't open a searcher... but I'm skeptical a user would need to 
be aware of such a distinction.  We do the latter but we could improve Solr to 
do the former someday and it ought not to change callers of our APIs like this.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.

Review Comment:
   IMO we shouldn't conflate commit options with optimize options with 
expungeDelete options (which have no options).
   
   Also this wording is common for LLMs... it conveys a _change_ (why it was 
introduced), instead of simply documenting it's purpose.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/CommitOptions.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.request;
+
+/**
+ * Encapsulates commit and optimize options for update requests. This record 
provides a type-safe
+ * way to specify commit parameters instead of using multiple boolean 
parameters in method
+ * signatures.
+ *
+ * @since Solr 10.0
+ */
+public record CommitOptions(
+    boolean waitSearcher,
+    boolean openSearcher,
+    boolean softCommit,
+    boolean expungeDeletes,
+    int maxOptimizeSegments) {
+
+  /** Compact constructor with validation. */
+  public CommitOptions {
+    if (maxOptimizeSegments < 1) {
+      throw new IllegalArgumentException(
+          "maxOptimizeSegments must be >= 1, got: " + maxOptimizeSegments);
+    }
+  }
+
+  /**
+   * Creates default commit options with: - waitSearcher: true - openSearcher: 
true - softCommit:
+   * false - expungeDeletes: false - maxOptimizeSegments: Integer.MAX_VALUE
+   */
+  public CommitOptions() {
+    this(true, true, false, false, Integer.MAX_VALUE);
+  }
+
+  /**
+   * Sets whether to wait for the searcher to be registered/visible. Returns a 
new CommitOptions
+   * instance with the updated value.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance for method chaining
+   */
+  public CommitOptions waitSearcher(boolean waitSearcher) {
+    return new CommitOptions(
+        waitSearcher, openSearcher, softCommit, expungeDeletes, 
maxOptimizeSegments);
+  }
+
+  /**
+   * Returns a new CommitOptions with the waitSearcher value changed.
+   *
+   * @param waitSearcher true to wait for searcher
+   * @return new CommitOptions instance with updated waitSearcher
+   */
+  public CommitOptions withWaitSearcher(boolean waitSearcher) {

Review Comment:
   pointless



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to