You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "ChaomingZhangCN (via GitHub)" <gi...@apache.org> on 2026/10/23 04:36:39 UTC
[PR] test(metrics): make histogram windowing tests deterministic [paimon-cpp]
ChaomingZhangCN opened a new pull request, #458:
URL: https://github.com/apache/paimon-cpp/pull/458
### Purpose
Fix the timing-dependent failure of `HistogramWindowingImplTest.TestResetWhenBeyondMaxSpan` seen in the [gcc-debug-aarch64 job for #453](https://github.com/apache/paimon-cpp/actions/runs/39885774312/job/120222877442?pr=453).
The test retains samples for only two 1 ms windows. A scheduling delay between `Add(3)` and `GetStats()` can legitimately expire the new sample, producing count/min/max/average of zero. Real sleeps also leave the other windowing tests sensitive to scheduling and window alignment.
Allow the internal `HistogramWindowingImpl` to accept a microsecond clock callback, defaulting to the existing system clock. Use it for initialization, updates, reads, and merges, and preserve it when cloning. Convert the windowing tests to manually advance time, removing all real sleeps and oversized timing margins.
This is an independent fix based on `main`, separate from the vector reader changes in #453.
### Tests
- All 10 windowing tests pass, including explicit expiry checks one microsecond before and exactly at `max_span`, reset at/beyond `max_span`, plain-histogram merge, and clone expiry using the same clock.
- All 10 windowing tests pass for 1,000 shuffled repetitions (10,000 test executions).
- All 19 tests from `histogram_test.cpp`, `histogram_windowing_test.cpp`, and `metrics_impl_test.cpp` pass, covering the production default clock through `MetricsImpl`.
- The same 19 tests pass with AddressSanitizer and UndefinedBehaviorSanitizer, with halt-on-error enabled.
- Changed-file pre-commit hooks and `git diff --check` pass.
Validation ran on macOS arm64 using C++17, `-Wall -Wextra -Werror`, and standalone executables compiled from the current worktree sources and existing GoogleTest/fmt/RapidJSON dependencies. The complete `paimon-common-test` target and Linux CI matrix were not run locally; the existing configured build belongs to a different checkout.
<details>
<summary>Local validation commands</summary>
Run from the new worktree root. Build outputs are outside the repository.
```bash
c++ -std=c++17 -Wall -Wextra -Werror -g -O0 -pthread -Iinclude -Isrc -isystem /tmp/paimon-197-vortex-build/googletest_ep-install/include src/paimon/common/metrics/histogram.cpp src/paimon/common/metrics/histogram_windowing.cpp src/paimon/common/metrics/histogram_windowing_test.cpp /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtest_maind.a /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtestd.a -o /tmp/paimon-histogram-clock.xEANiG/histogram-windowing-test
/tmp/paimon-histogram-clock.xEANiG/histogram-windowing-test --gtest_filter='HistogramWindowingImplTest.*'
/tmp/paimon-histogram-clock.xEANiG/histogram-windowing-test --gtest_filter='HistogramWindowingImplTest.*' --gtest_repeat=1000 --gtest_shuffle --gtest_break_on_failure > /tmp/paimon-histogram-clock.xEANiG/repeat.log
c++ -std=c++17 -Wall -Wextra -Werror -Wno-unused-parameter -g -O0 -pthread -DRAPIDJSON_HAS_STDSTRING=1 -Iinclude -Isrc -isystem /tmp/paimon-197-vortex-build/googletest_ep-install/include -isystem /tmp/paimon-197-vortex-build/rapidjson_ep-install/include -isystem /opt/homebrew/include src/paimon/common/metrics/histogram.cpp src/paimon/common/metrics/histogram_windowing.cpp src/paimon/common/metrics/metrics_impl.cpp src/paimon/common/utils/status.cpp src/paimon/common/metrics/histogram_test.cpp src/paimon/common/metrics/histogram_windowing_test.cpp src/paimon/common/metrics/metrics_impl_test.cpp /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtest_maind.a /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtestd.a /opt/homebrew/lib/libfmt.a -o /tmp/paimon-histogram-clock.xEANiG/metrics-test
/tmp/paimon-histogram-clock.xEANiG/metrics-test
c++ -std=c++17 -Wall -Wextra -Werror -Wno-unused-parameter -g -O1 -pthread -fsanitize=address,undefined -fno-omit-frame-pointer -DRAPIDJSON_HAS_STDSTRING=1 -Iinclude -Isrc -isystem /tmp/paimon-197-vortex-build/googletest_ep-install/include -isystem /tmp/paimon-197-vortex-build/rapidjson_ep-install/include -isystem /opt/homebrew/include src/paimon/common/metrics/histogram.cpp src/paimon/common/metrics/histogram_windowing.cpp src/paimon/common/metrics/metrics_impl.cpp src/paimon/common/utils/status.cpp src/paimon/common/metrics/histogram_test.cpp src/paimon/common/metrics/histogram_windowing_test.cpp src/paimon/common/metrics/metrics_impl_test.cpp /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtest_maind.a /tmp/paimon-197-vortex-build/googletest_ep-install/lib/libgtestd.a /opt/homebrew/lib/libfmt.a -o /tmp/paimon-histogram-clock.xEANiG/metrics-sanitized-test
UBSAN_OPTIONS=halt_on_error=1 ASAN_OPTIONS=halt_on_error=1 /tmp/paimon-histogram-clock.xEANiG/metrics-sanitized-test
PATH=/tmp/paimon-rebase-20261020-tools/bin:$PATH pre-commit run --files src/paimon/common/metrics/histogram_windowing.h src/paimon/common/metrics/histogram_windowing.cpp src/paimon/common/metrics/histogram_windowing_test.cpp
git diff --check
```
</details>
### API and Format
No changes to public headers under `include/paimon/`, storage formats, or protocols. The optional clock callback belongs to the internal histogram implementation; production uses the same system clock and windowing rules as before.
### Documentation
No user-facing feature or configuration change; no documentation update is needed.
### Generative AI tooling
Generated-by: OpenAI Codex CLI 0.163.0-alpha.11.1
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(metrics): make histogram windowing tests deterministic [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 merged PR #458:
URL: https://github.com/apache/paimon-cpp/pull/458
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org