You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "epugh (via GitHub)" <gi...@apache.org> on 2026/01/16 19:56:06 UTC
[PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
epugh opened a new pull request, #4058:
URL: https://github.com/apache/solr/pull/4058
https://issues.apache.org/jira/browse/SOLR-7003
# Description
This removes completely the unused boolean waitFlush from Solr Client and the tests. This would be Solr 11 only, so applied to the `main` branch.
In a seperate PR we will deprecate the property in the client, but leave the client signature as is. We will remove it from the server and tests however since it doesn't do anything. That will go to `branch_10x` and ship in Solr 10.1.
# Solution
Refactoring.
# Tests
Updated the tests and re-run them.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4058:
URL: https://github.com/apache/solr/pull/4058#issuecomment-3764369858
I've got a new branch that is going *much better*....
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh closed pull request #4058: SOLR-7003: Remove unused boolean waitFlush from SolrClients
URL: https://github.com/apache/solr/pull/4058
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4058:
URL: https://github.com/apache/solr/pull/4058#issuecomment-3764284337
> As this is the removal PR, I am curious -- did you remove the parameter with IntelliJ or did you use an LLM? Or worse, did you use IntelliJ like a text editor and do it manually? IntelliJ freaking excels at refactoring; you can trust it will mechanically / assuredly make the change.
I did BOTH of those things... ;-(. And literally on the plane home I was thinking "Something is going wrong with my approach". I tried IntellJ and it doesn't want to refactor based on the changed method collides iwth another method witht he same signature. This was mentioned in hte ORIGINAL JIRA posting... I did discover the `CommitUpdateCommand` which encapsulates all these parameters. So I am going to take the slightly larger surgury to see if instead of passing all those paremters around, maybe I can instead just use this class instead? (Inspired by what @HoustonPutman is doing on the collection api...)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4058:
URL: https://github.com/apache/solr/pull/4058#discussion_r2699860421
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##########
@@ -247,7 +247,7 @@ 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, true, true);
Review Comment:
err, you are right, I still have some changes to AbstractUpdateRequest to do.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4058:
URL: https://github.com/apache/solr/pull/4058#discussion_r2699817059
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##########
@@ -247,7 +247,7 @@ 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, true, true);
Review Comment:
The soft commit parameter is being lost in this refactoring. The method is named `softCommit` and line 226 has a comment 'Calling soft commit to make sub shard updates visible'. The old code was `setAction(AbstractUpdateRequest.ACTION.COMMIT, false, true, true)` where `softCommit=true`. The new 3-parameter version defaults `softCommit` to `false`. Change to `ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true)` to maintain the soft commit behavior.
```suggestion
ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true);
```
##########
solr/core/src/test/org/apache/solr/cloud/RecoveryAfterSoftCommitTest.java:
##########
@@ -91,7 +91,7 @@ public void test() throws Exception {
// soft-commit so searchers are open on un-committed but flushed segment files
AbstractUpdateRequest request =
- new UpdateRequest().setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true);
+ new UpdateRequest().setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true);
Review Comment:
The soft commit parameter is being lost in this refactoring. The old code called `setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true)` where the fourth parameter was `softCommit=true`. The new 3-parameter version calls the overload that defaults `softCommit` to `false` (see AbstractUpdateRequest line 40 which calls line 50). The comment on line 92-93 explicitly states this should be a soft-commit. Change to `new UpdateRequest().setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true)` to preserve the soft commit behavior.
```suggestion
new UpdateRequest().setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true, true);
```
##########
solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java:
##########
@@ -295,7 +295,7 @@ void addCommit(UpdateRequest ureq, CommitUpdateCommand cmd) {
if (cmd == null) return;
ureq.setAction(
cmd.optimize ? AbstractUpdateRequest.ACTION.OPTIMIZE : AbstractUpdateRequest.ACTION.COMMIT,
- false,
+ true, // waitFlush - ignored
Review Comment:
The comment 'waitFlush - ignored' is misleading since this parameter is no longer named waitFlush in the underlying AbstractUpdateRequest.setAction() method - it's still a required parameter at the AbstractUpdateRequest level. A more accurate comment would be 'waitFlush (not used by server but required by API)' or similar to clarify the parameter's status.
```suggestion
true, // waitFlush (not used by server but required by API)
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4058:
URL: https://github.com/apache/solr/pull/4058#discussion_r2699850744
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##########
@@ -247,7 +247,7 @@ 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, true, true);
Review Comment:
I don't think this is true...
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4058:
URL: https://github.com/apache/solr/pull/4058#discussion_r2701225145
##########
changelog/unreleased/SOLR-7003.yml:
##########
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Remove unused waitFlush boolean from SolrClients
Review Comment:
should clarify it's association to `commit`
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4058:
URL: https://github.com/apache/solr/pull/4058#issuecomment-3768970394
This PR is not going well, but new appraoch is!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7003: Remove unused boolean waitFlush from SolrClients [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4058:
URL: https://github.com/apache/solr/pull/4058#issuecomment-3764493432
Sometimes you need to be a little clever about breaking down a refactoring into multiple baby steps that you can feed to IntelliJ, when you discover the roadblock you saw. For example if there will be a signature collection, then just change the method that's going away to call the new one (manually; very simple), then tell IntelliJ to inline the method, which it will happily do.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org