Skip to content

[fix](regression) Stabilize rowsets timestamp filters#65050

Merged
mrhhsg merged 1 commit into
apache:masterfrom
mrhhsg:fix/stable-rowsets-regression
Jul 1, 2026
Merged

[fix](regression) Stabilize rowsets timestamp filters#65050
mrhhsg merged 1 commit into
apache:masterfrom
mrhhsg:fix/stable-rowsets-regression

Conversation

@mrhhsg

@mrhhsg mrhhsg commented Jun 30, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Problem Summary: The system rowsets regressions used second-level current timestamps as lower bounds for NEWEST_WRITE_TIMESTAMP. In one test, the timestamp was captured after the table was created and could filter out the initial rowset on a boundary race. In the cloud-only scan variant, the timestamp could be equal to the create-table rowset timestamp and unexpectedly include version 0-1. Capture stable lower bounds on the intended side of table creation for each case, so the tests still exercise timestamp filters while avoiding second-level boundary races.

Release note

None

Check List (For Author)

  • Test:
    • Regression test: ./run-regression-test.sh --run -d query_p0/system -s test_query_sys_rowsets
    • Regression test: ./run-regression-test.sh --run -d query_p0/system -s test_query_sys_rowsets (200 consecutive runs)
    • Regression test: ./run-regression-test.sh --run -d query_p0/system -s test_query_sys_scan_rowsets (local non-cloud guard path)
    • Regression test: ./run-regression-test.sh --run -d query_p0/system -s test_query_sys_rowsets
  • Behavior changed: No
  • Does this need documentation: No

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@mrhhsg

mrhhsg commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review result: requesting changes for one test-stability gap.

Critical checkpoint conclusions:

  • Goal/test proof: the changed test now captures a stable lower bound before table creation and keeps the expected rowset versions unchanged.
  • Scope: the edit is small, but the same timestamp-boundary pattern remains in a parallel rowsets regression.
  • Concurrency/lifecycle/config/compatibility/persistence: no product-code, config, protocol, lifecycle, or storage-format changes in this PR.
  • Parallel paths: M-1 identifies the remaining cloud-only rowsets scan test path with the same second-level >= now race.
  • Test results: PR body reports query_p0/system test_query_sys_rowsets runs including 20 consecutive runs; I did not run local regression here because this checkout has no thirdparty/installed.
  • User focus: no additional focus was supplied.

Subagent conclusions: optimizer-rewrite and tests-session-config both reported no candidates in their initial passes. After M-1 was added to the ledger/proposed comment set, convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS.

### What problem does this PR solve?

Issue Number: None

Problem Summary: The system rowsets regressions used second-level current timestamps as lower bounds for NEWEST_WRITE_TIMESTAMP. In one test, the timestamp was captured after the table was created and could filter out the initial rowset on a boundary race. In the cloud-only scan variant, the timestamp could be equal to the create-table rowset timestamp and unexpectedly include version 0-1. Capture stable lower bounds on the intended side of table creation for each case, so the tests still exercise timestamp filters while avoiding second-level boundary races.

### Release note

None

### Check List (For Author)

- Test:
    - Regression test: doris-local-regression --network 10.26.20.3/24 all -d query_p0/system -s test_query_sys_rowsets
    - Regression test: doris-local-regression --network 10.26.20.3/24 run -d query_p0/system -s test_query_sys_rowsets (200 consecutive runs)
    - Regression test: doris-local-regression --network 10.26.20.3/24 run -d query_p0/system -s test_query_sys_scan_rowsets (local non-cloud guard path)
    - Regression test: doris-local-regression --network 10.26.20.3/24 run -d query_p0/system -s test_query_sys_rowsets
- Behavior changed: No
- Does this need documentation: No
@mrhhsg

mrhhsg commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

/review

@mrhhsg mrhhsg force-pushed the fix/stable-rowsets-regression branch from cceab75 to f4c3543 Compare June 30, 2026 15:11
@mrhhsg mrhhsg changed the title [fix](regression) Stabilize rowsets timestamp filter [fix](regression) Stabilize rowsets timestamp filters Jun 30, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review completed for PR 65050 at f4c3543.

I did not find any new blocking issue or any new valuable inline comment beyond the existing thread that has already been addressed by the current patch.

Critical checkpoint conclusions:

  • Goal and proof: the PR targets second-level NEWEST_WRITE_TIMESTAMP boundary flakiness in the rowsets regressions. The normal rowsets test now captures a lower bound before table creation, preserving the expected rowsets4 output that includes 0-1 through 5-5; the cloud scan variant now waits past the create-table rowset second before capturing the lower bound, preserving the expected rowsets4 output that excludes 0-1.
  • Scope and focus: the live GitHub PR file list and compare API show only the two rowsets regression suites. The change is small and focused on timestamp filter stabilization.
  • Parallel paths: the sibling cloud-only scan test is now updated, so the existing inline concern about leaving test_query_sys_scan_rowsets.groovy flaky is covered by the current head.
  • Tests and expected results: the expected-output files already encode the intended difference between the local and cloud scan variants. I verified the changed queries against those expectations and the BE rowsets scanner path that exposes newest_write_timestamp as a DATETIME from Unix-second rowset metadata.
  • Concurrency, lifecycle, configs, compatibility, transactions, and observability: this is regression-test-only code and does not add runtime product state, config, persistence, protocol, or logging behavior.
  • Performance: the added 1.1s sleep is limited to the cloud-only regression path and is directly tied to the second-level timestamp boundary being tested.
  • Hygiene: git diff --check is clean for the true PR parent-to-head range over the changed files.

User focus: no additional user-provided review focus was present.

Subagent conclusions:

  • optimizer-rewrite reported NO_NEW_VALUABLE_FINDINGS; no optimizer/rewrite candidate became an inline comment.
  • tests-session-config reported NO_NEW_VALUABLE_FINDINGS; no test/config candidate became an inline comment.
  • Main merged MAIN-DUP-001 as a duplicate/resolution note for the existing inline thread about the cloud-only sibling test.
  • Convergence round 1 ended with both live subagents reporting NO_NEW_VALUABLE_FINDINGS for the same empty proposed final comment set after the last ledger update.

@mrhhsg mrhhsg marked this pull request as ready for review July 1, 2026 01:05
@mrhhsg

mrhhsg commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

run buildall

@HappenLee HappenLee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@mrhhsg mrhhsg merged commit 9d93a33 into apache:master Jul 1, 2026
37 checks passed
@mrhhsg mrhhsg deleted the fix/stable-rowsets-regression branch July 1, 2026 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants