Use DEFAULT_MAX_PERMITS constant in getSharedArenaMaxPermitsSysprop

[Huaixinww]

This PR is a follow-up to #15078
The previous change in #15078 intended to reduce the default value of SharedArenaMaxPermits from 1024 to 64 by updating RefCountedSharedArena.DEFAULT_MAX_PERMITS.

However, the change did not take effect because of a magic number in the getSharedArenaMaxPermitsSysprop() method:

private static int getSharedArenaMaxPermitsSysprop() {
    int ret = 1024; // default value

This magic number effectively overrides the DEFAULT_MAX_PERMITS constant, making the intended change from the previous PR ineffective.

This PR fixes the issue by replacing the magic number 1024 with the DEFAULT_MAX_PERMITS constant. This ensures that the default value is sourced from the constant as intended.

If there are other considerations here, please let me know.

[Huaixinww]

hello @ChrisHegarty and @uschindler
We encountered an issue similar to #15068. After some analysis, we realized that the change from PR #15078 did not fully address the problem as intended. This new PR proposes what we believe is the correct fix.

We would appreciate it if you could review it to confirm whether this is indeed the reason.

[Huaixinww]

One more thing. We found a related discussion on the mailing list here:
https://lists.apache.org/thread/4lqh5w9mxm4ffr5kxlxhh06d9gdv3gto

In the thread, Uwe Schindler mentioned a potential performance degradation in update-heavy scenarios if the limit is set back to 1. I've searched through related issues but couldn't find any specific benchmarks comparing these values.

I was wondering: has the Lucene project conducted any specific performance tests on this? For instance, a comparison of performance under heavy updates with the SharedArenaMaxPermits limit set to 1024, 64, and 1?

Any insights on this would be greatly appreciated.

[uschindler]

Hi thanks for the correction, I noticed that problem, too.

About your question: You can't make a good benchamrk because that depends on load used, number of Threads and also JVM version. It is highly dependent on the used software. A major problem is Solr, which has some additional issues because it uses wrong IOContext to read "state" or "metadata files" during replication. It should use READ_ONCE there, currently it uses DEFAULT, which makes the problem get even larger.

[uschindler]

Solr issues about the metadata files with wrong context: https://issues.apache.org/jira/browse/SOLR-17375

Please add a changes entry for Lucene 10.5. I will also check if it needs to be backported to 9.x as this is urgent and maybe Solr may need to update.

[Huaixinww]

Solr issues about the metadata files with wrong context: https://issues.apache.org/jira/browse/SOLR-17375

Please add a changes entry for Lucene 10.5. I will also check if it needs to be backported to 9.x as this is urgent and maybe Solr may need to update.

Thanks for ur feedback! I've added a changelog entry for this PR. Please let me know if there's anything else I need to change.

[uschindler]

Solr issues about the metadata files with wrong context: https://issues.apache.org/jira/browse/SOLR-17375
Please add a changes entry for Lucene 10.5. I will also check if it needs to be backported to 9.x as this is urgent and maybe Solr may need to update.

Thanks for ur feedback! I've added a changelog entry for this PR. Please let me know if there's anything else I need to change.

Can you move the changes entry to the 10.5 section, the changelog bot detected v 11.0?

[uschindler]

In addition there seem to be some test problems. Maybe merge up to main branch's head. Looks like something is broken in the branch you're using. Please make sure it is latest stage.

[Huaixinww]

Got it. Moved the change entry to 10.5 and synced with main.

[uschindler]

Hi,
the test failures are coming from main branch. It's not your fault. Let's ignore those.

[uschindler]

Perfect, will merge soon. Looks like @ChrisHegarty is on vacation.

[uschindler]

We I'll cherry pick in 10x branch soon. No further action required.

[uschindler]

Hi, backport is not so easy, because the constant in RefCountedSharedArena is not accessible in the MR-JAR framework in Lucene 10.x.

I will possibly backport with the hardcoded constant. That may have been the main reason why the hardcoded constant was used.

[uschindler]

Here is the backport PR. The code previously was a bit stupid (relic from 9.x branch), so I moved the check code into the provider: #15930

[uschindler]

Backport done.

[uschindler]

The backport to 9.12 branch is here as the bug also exists there and was already released last September: #15931

[uschindler]

Backported to 9.12 branch.