Skip to content

SOLR-18089: Configure zookeeper.maxCnxns for embeddedZkServer#4173

Open
openworld-maker wants to merge 2 commits intoapache:mainfrom
openworld-maker:codex/SOLR-18089-embedded-zk-maxcnxns
Open

SOLR-18089: Configure zookeeper.maxCnxns for embeddedZkServer#4173
openworld-maker wants to merge 2 commits intoapache:mainfrom
openworld-maker:codex/SOLR-18089-embedded-zk-maxcnxns

Conversation

@openworld-maker
Copy link
Contributor

What

Fixes SOLR-18089 by explicitly configuring ZooKeeper's zookeeper.maxCnxns system property to 0 for embedded ZooKeeper startup when it is not already set.

Why

Embedded ZK startup emits:
"maxCnxns is not configured, using default value 0."

ZooKeeper uses zookeeper.maxCnxns with default 0. Setting it explicitly suppresses the warning while preserving behavior.

Changes

  • Added constants in SolrZkServer:
    • ZK_MAX_CNXNS_PROPERTY = "zookeeper.maxCnxns"
    • ZK_MAX_CNXNS_DEFAULT = "0"
  • Added ensureZkMaxCnxnsConfigured() and invoked it in:
    • SolrZkServer.start() before ZooKeeper startup
    • ZkTestServer.run(...) before server initialization in test framework
  • Guarded behavior: only sets property when currently unset (does not override user-provided value).

Tests

  • Added ZkTestServerTest#testEmbeddedZkServerSetsMaxCnxnsDefaultWhenUnset:
    • clears zookeeper.maxCnxns
    • starts embedded ZkTestServer
    • asserts property equals "0"
    • restores prior property value in finally

Verification

  • :solr:test-framework:test --tests org.apache.solr.cloud.ZkTestServerTest passed.
  • Full tidy check was attempted but hit unrelated flaky failures in solrj tests (CloudSolrClientCacheTest).
  • Ran equivalent focused validation for touched modules:
    • tidy
    • :solr:core:check -x test
    • :solr:test-framework:check -x test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, you can merely observe (once, manually) that the log message about this matter has disappeared. Tests need to be maintained and add a cost in perpetuity that I don't think is warranted for this matter.

Comment on lines +216 to +218
if (System.getProperty(ZK_MAX_CNXNS_PROPERTY) == null) {
System.setProperty(ZK_MAX_CNXNS_PROPERTY, ZK_MAX_CNXNS_DEFAULT);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets use `System.getProperties().putIfAbsent(...) instead of multiple operaitons


public static final String ZK_WHITELIST_PROPERTY = "zookeeper.4lw.commands.whitelist";
public static final String ZK_MAX_CNXNS_PROPERTY = "zookeeper.maxCnxns";
public static final String ZK_MAX_CNXNS_DEFAULT = "0";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment what this value semantically means. Infinite?

@github-actions github-actions bot removed the tests label Mar 3, 2026
@openworld-maker
Copy link
Contributor Author

@dsmiley requested updates are done:

  • Removed the added ZkTestServer test (per your guidance to avoid perpetually-maintained test cost for this case).
  • Updated ensureZkMaxCnxnsConfigured to use System.getProperties().putIfAbsent.
  • Added a clarifying comment that ZooKeeper zookeeper.maxCnxns=0 means no limit.

Thanks for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants