You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "syahra712 (via GitHub)" <gi...@apache.org> on 2026/10/17 17:56:25 UTC
[PR] Read collection-stats-sample-interval for SolrCollectionStats [solr-orbit]
syahra712 opened a new pull request, #137:
URL: https://github.com/apache/solr-orbit/pull/137
### Description
`docs/reference/telemetry.md` documents `--telemetry-params collection-stats-sample-interval:60` for the always-on collection stats device. However, `_create_solr_telemetry_devices` built `SolrCollectionStats` without a `sample_interval_s`, so the device always polled every 30 s.
This PR reads `collection-stats-sample-interval` from the telemetry params, with a default of 30, and passes it to `SolrCollectionStats`. Like `ShardStats` with `shard-stats-sample-interval`, `SolrCollectionStats` now raises `SystemSetupError` for values <= 0. Otherwise a zero interval would make `_poll_loop` poll Solr in a tight loop.
### Issues Resolved
Fixes #136 (found while looking into #5)
### Testing
- [x] New functionality includes testing
- `tests/worker_coordinator/worker_coordinator_test.py::WorkerCoordinatorTests::test_collection_stats_uses_sample_interval_param` sets `collection-stats-sample-interval: 7` and checks the created device's interval. On `main` it fails with `7 != 30.0`.
- `tests/unit/solr/test_telemetry.py::TestSolrCollectionStats::test_rejects_non_positive_sample_interval` checks that 0 and -5 are rejected.
- The full `tests/` suite passes locally on Python 3.12 (1176 passed, 5 skipped), and `ruff check .` passes. I haven't run the integration suite (`it/`).
This touches the same list in `_create_solr_telemetry_devices` as #135, but different lines, so either can merge first.
---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
--
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] Read collection-stats-sample-interval for SolrCollectionStats [solr-orbit]
Posted by "syahra712 (via GitHub)" <gi...@apache.org>.
syahra712 commented on PR #137:
URL: https://github.com/apache/solr-orbit/pull/137#issuecomment-6325572262
Done in 1015b71f. `SolrCollectionStats` now takes `telemetry_params` and reads `collection-stats-sample-interval` itself, like `ShardStats`. It still defaults to 30 s and rejects values <= 0. The coordinator just passes `telemetry_params` through. I added a unit test for the default and for the param. The full suite passes (1178 passed, 5 skipped).
--
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] Read collection-stats-sample-interval for SolrCollectionStats [solr-orbit]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #137:
URL: https://github.com/apache/solr-orbit/pull/137#issuecomment-6311012709
Yes, let's standardize on passing `telemetry_params` for consistency and smoother to add params in the future without method signature changes.
--
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] Read collection-stats-sample-interval for SolrCollectionStats [solr-orbit]
Posted by "syahra712 (via GitHub)" <gi...@apache.org>.
syahra712 commented on PR #137:
URL: https://github.com/apache/solr-orbit/pull/137#issuecomment-6290384882
Good question. It's a gap in the port rather than a new design. `SolrCollectionStats` (like the other `Solr*Stats` devices) was written for Solr in the initial port (57387ed4) and has no OpenSearch Benchmark counterpart. So there's no upstream wiring to copy for this device. The closest upstream pattern is OSB's `NodeStats`/`ShardStats`: they receive `telemetry_params`, read `<device>-sample-interval` themselves, and raise `SystemSetupError` for values <= 0. In solr-orbit, the ported `ShardStats` still works that way. The always-on Solr devices are built in `_create_solr_telemetry_devices` without the params, which is why the documented `collection-stats-sample-interval` was never read.
This PR copies the upstream validation but passes the value at the call site. If you'd rather match OSB exactly, I can pass `telemetry_params` into `SolrCollectionStats.__init__` and read it there, as `ShardStats` does. Just say which you prefer.
I also merged main to fix the conflict with the `ClusterEnvironmentInfo` test (both tests kept). The full `tests/` suite passes: 1177 passed, 5 skipped.
--
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] Read collection-stats-sample-interval for SolrCollectionStats [solr-orbit]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #137:
URL: https://github.com/apache/solr-orbit/pull/137#issuecomment-6278666180
Is this solution informed by how Opensearch Benchmarks wires things or is it a novel fix? We might want to consult “upstream” to find out where the solr-port went wrong.
--
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