Skip to content

Conversation

@dl239
Copy link
Collaborator

@dl239 dl239 commented Nov 10, 2025

close #3935

@github-actions github-actions bot added the storage-engine openmldb storage engine. nameserver & tablet label Nov 10, 2025
@dl239 dl239 requested a review from Copilot November 10, 2025 08:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the GC (Garbage Collection) implementation for multi-index TTL handling and adds test coverage for the fix. The bug was that GcAllType was using the raw abs_ttl value directly instead of calculating the correct expire_time by subtracting it from the current time (with offset).

  • Corrected expire_time calculation in GcAllType method to match the pattern used in single-TTL ExecuteGc
  • Added a new test TestGcAllType to validate GC behavior with multiple timestamp indices and different TTL values
  • Added debug logging to track GC operations on keys

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/storage/segment.cc Fixed expire_time calculation in GcAllType method for all TTL types (kAbsoluteTime, kAbsAndLat, kAbsOrLat) by properly computing cur_time - ttl_offset_ - abs_ttl instead of using abs_ttl directly
src/storage/segment_test.cc Added comprehensive test case for GC with multiple timestamp indices to verify the corrected expire_time logic works properly with different TTL configurations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

SDK Test Report

102 files  102 suites   2m 8s ⏱️
359 tests 345 ✅ 14 💤 0 ❌
487 runs  473 ✅ 14 💤 0 ❌

Results for commit 806b927.

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 89.85507% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.33%. Comparing base (2ee066e) to head (806b927).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/storage/segment_test.cc 89.47% 6 Missing ⚠️
src/storage/segment.cc 91.66% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #4057       +/-   ##
=============================================
+ Coverage     41.61%   75.33%   +33.71%     
  Complexity      711      711               
=============================================
  Files           196      763      +567     
  Lines         11789   143666   +131877     
  Branches       1538     2076      +538     
=============================================
+ Hits           4906   108225   +103319     
- Misses         6580    35138    +28558     
  Partials        303      303               

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Contributor

Linux Test Report

    60 files     253 suites   1h 54m 5s ⏱️
13 630 tests 13 623 ✅ 7 💤 0 ❌
19 352 runs  19 345 ✅ 7 💤 0 ❌

Results for commit 806b927.

@dl239 dl239 merged commit f14b323 into 4paradigm:main Nov 11, 2025
30 checks passed
@dl239 dl239 deleted the fix/execute_gc branch November 11, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

storage-engine openmldb storage engine. nameserver & tablet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

segment gc when ts_cnt > 1

1 participant