[CORE-13570] Deflake test_timequery_with_local_gc#28070
[CORE-13570] Deflake test_timequery_with_local_gc#28070WillemKauf merged 2 commits intoredpanda-data:devfrom
test_timequery_with_local_gc#28070Conversation
This test intended to disable local retention, but did not consider the property `retention.local.target.ms` for a tiered storage enabled topic. Previously, local retention would be enforced by broker time, but after PR redpanda-data#27383, the test became flakey due to the now "active" retention enforcement. Set `retention.local-target.ms=-1` to disable time-based retention for tiered storage enabled topics for this test.
The request `log_segment_size` in `test_timequery_with_local_gc` was actually being ignored at start-up due to the bounded property checks, manifesting as a `WARN` level log line. Disable bounded property checks so the test can run as requested.
| rpk_rr_cluster.create_topic(self.topic_name, config=conf) | ||
| except: | ||
| self.logger.warn(f"Failed to create a read-replica topic") | ||
| self.logger.warn("Failed to create a read-replica topic") |
There was a problem hiding this comment.
ruff linter unnecessary f-string removal
There was a problem hiding this comment.
Pull Request Overview
Deflake the test_timequery_with_local_gc by disabling local time-based retention for tiered-storage topics and allowing a smaller-than-minimum log_segment_size during the test.
- Disable time-based local retention by setting retention.local.target.ms to -1 for the test topic
- Set __REDPANDA_TEST_DISABLE_BOUNDED_PROPERTY_CHECKS before starting the cluster to permit sub-minimum log_segment_size
- Minor cleanup of a log message string
| "redpanda.remote.read": "true", | ||
| "redpanda.remote.write": "true", | ||
| "retention.local.target.bytes": local_retention, | ||
| # See comment about disabling `retention.ms` below. |
There was a problem hiding this comment.
The inline comment references disabling retention.ms, but the code is setting retention.local.target.ms. Please update the comment to reflect the local retention setting to avoid confusion. For example: 'Disable time-based local retention for tiered storage by setting retention.local.target.ms to -1.'
| # See comment about disabling `retention.ms` below. | |
| # Disable time-based local retention for tiered storage by setting retention.local.target.ms to -1. |
| except: | ||
| self.logger.warn(f"Failed to create a read-replica topic") | ||
| self.logger.warn("Failed to create a read-replica topic") |
There was a problem hiding this comment.
Avoid a bare except, which swallows KeyboardInterrupt/SystemExit and makes debugging harder. Catch Exception explicitly and log the exception details.
| rpk_rr_cluster.create_topic(self.topic_name, config=conf) | ||
| except: | ||
| self.logger.warn(f"Failed to create a read-replica topic") | ||
| self.logger.warn("Failed to create a read-replica topic") |
There was a problem hiding this comment.
logger.warn is deprecated; use logger.warning instead. Consider including exc_info to capture the exception context.
| self.logger.warn("Failed to create a read-replica topic") | |
| self.logger.warning("Failed to create a read-replica topic", exc_info=True) |
CI test resultstest results on build#74273
|
|
/ci-repeat 5 |
| self.redpanda.set_environment( | ||
| {"__REDPANDA_TEST_DISABLE_BOUNDED_PROPERTY_CHECKS": "ON"} | ||
| ) |
There was a problem hiding this comment.
ahh the warning was on bootup, not from something later like alter topic config. i think the later will fail, but indeed this just seems to warn silently on bootup.
Fixes https://redpandadata.atlassian.net/issues/CORE-13570.
This test intended to disable local time-based retention, but did not consider the property
retention.local.target.msfor a tiered storage enabled topic.Previously, local retention would be enforced by broker time, but after PR #27383, the test became flakey due to the now "active" retention enforcement.
Set
retention.local-target.ms=-1to disable time-based retention for tiered storage enabled topics for this test.Backports Required
Release Notes