nick-boss-tech opened a new pull request, #5083: URL: https://github.com/apache/solr/pull/5083
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-11483 ## What happens today **With numRecordsToKeep set, the update log can keep fewer records than configured.** The update log removes its oldest transaction log file once it holds `maxNumLogsToKeep` files, even when fewer than `numRecordsToKeep` records would remain ([UpdateLog.java#L764-L765](https://github.com/nick-boss-tech/solr/blob/4431a250f6650494f8d87bd689a7f45d5b4835ab/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L764-L765)). The default `maxNumLogsToKeep` is 10. A config that raises `numRecordsToKeep` but leaves the file limit at its default can therefore keep fewer records than it asks for. ## What this change does **When numRecordsToKeep is set and maxNumLogsToKeep is not, the file limit defaults to 1000 instead of 10.** - The default depends on whether the `numRecordsToKeep` key is present in the config. A config that sets it to the default 100 also gets 1000 ([UpdateLog.java#L386-L393](https://github.com/nick-boss-tech/solr/blob/4431a250f6650494f8d87bd689a7f45d5b4835ab/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L386-L393)). - An explicit `maxNumLogsToKeep` is used as before. - Default installs are not affected. No shipped config under `solr/server` sets either setting. - The reference guide states the new default ([commits-transaction-logs.adoc#L274](https://github.com/nick-boss-tech/solr/blob/4431a250f6650494f8d87bd689a7f45d5b4835ab/solr/solr-ref-guide/modules/configuration-guide/pages/commits-transaction-logs.adoc#L274)). ## Proof **`TestRecovery` passes at this head, and its new test fails on the base code.** - Test class `TestRecovery` passes 21 of 21 at this head (verified 2026-10-06). - New test [`testMaxNumLogsToKeepDefaultFollowsNumRecordsToKeep`](https://github.com/nick-boss-tech/solr/blob/4431a250f6650494f8d87bd689a7f45d5b4835ab/solr/core/src/test/org/apache/solr/search/TestRecovery.java#L404-L412). It checks three cases: the default stays 10, `numRecordsToKeep` alone gives 1000, and an explicit `maxNumLogsToKeep` of 7 is kept. On base production, `TestRecovery` runs 21 tests with exactly one failure: `expected:<1000> but was:<10>`. That is the 1000 check. ## A choice to check **Raise the file limit to 1000 when numRecordsToKeep is set, or keep 10 and document it.** The implemented route raises the default for any config that sets numRecordsToKeep. The alternative keeps the default at 10 and documents the limit, which the ticket allows if maintainers object. With that alternative, a config that sets numRecordsToKeep stays cut short by the file limit. Was 1000 the right default? ## Limits - Retention grows. With numRecordsToKeep set and maxNumLogsToKeep unset, up to 1000 log files can be kept instead of 10. This change does not measure the disk cost. - The trigger is the presence of the key. How an empty or unresolved value is parsed was not traced. - The new test creates an `UpdateLog` and does not close it. I can change that on request. Changelog: `changelog/unreleased/SOLR-11483-ulog-max-logs-default.yml` ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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]
