support iscp

[LeftHandCold]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #22296

What this PR does / why we need it:

support iscp

---

PR Type

Enhancement

---

Description

• Add ISCP (In-Storage Checkpoint) table support to garbage collection

• Implement custom ISCP tables function for testing scenarios

• Enhance checkpoint-based GC filtering with ISCP table awareness

• Add comprehensive test coverage for GC with custom ISCP tables

---

Diagram Walkthrough

flowchart LR
  A["checkpointCleaner"] --> B["iscpTablesFunc"]
  B --> C["ExecuteGlobalCheckpointBasedGC"]
  C --> D["MakeSnapshotAndPitrFineFilter"]
  D --> E["ISCP-aware filtering"]
  F["Test"] --> G["WithISCPTablesFunc"]
  G --> A

Loading

File Walkthrough

	Relevant files
Enhancement	checkpoint.go Add ISCP tables support to checkpoint cleaner                        pkg/vm/engine/tae/db/gc/v3/checkpoint.go Add iscpTablesFunc field to checkpointCleaner struct Implement WithISCPTablesFunc option for configuring ISCP tables Add ISCPTables() method to retrieve ISCP tables Pass ISCP tables to GC execution methods +19/-0    exec_v1.go Implement ISCP-aware GC filtering logic                                    pkg/vm/engine/tae/db/gc/v3/exec_v1.go Add iscpTables field to CheckpointBasedGCJob struct Update constructor to accept ISCP tables parameter Enhance MakeSnapshotAndPitrFineFilter with ISCP-aware filtering logic Add conditional GC logic based on ISCP table timestamps +30/-0    window.go Update GC window to support ISCP tables                                    pkg/vm/engine/tae/db/gc/v3/window.go Add iscpTables parameter to ExecuteGlobalCheckpointBasedGC method Pass ISCP tables to checkpoint-based GC job constructor +2/-0
Tests	db_test.go Add test coverage for ISCP GC functionality                            pkg/vm/engine/tae/db/test/db_test.go Add comprehensive test TestGCWithCustomISCPTables Test GC behavior with custom ISCP tables configuration Verify proper integration of ISCP functionality in GC system +95/-0

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 22296 - Partially compliant Compliant requirements: • Support compacted PITR for ISCP tables/databases • Keep CN-Created and Appendable-Objects for a specified time • Pin fewer objects compared to PITR Non-compliant requirements: • Support compacted PITR for CDC tables/databases (only ISCP is implemented) Requires further human verification: • Verification that the implementation correctly handles long duration settings • Testing with actual CDC scenarios to ensure proper integration

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Error The ISCP filtering logic has a potential bug where deleteTS.IsEmpty() && deleteTS.LT(&iscpTS) will always be false since an empty timestamp cannot be less than another timestamp. This condition should likely be !deleteTS.IsEmpty() && deleteTS.LT(&iscpTS). if (deleteTS.IsEmpty() && deleteTS.LT(&iscpTS)) || createTS.GT(&iscpTS) { continue } Inconsistent Logic The ISCP filtering logic differs between the two code paths - one checks !entry.dropTS.IsEmpty() && entry.dropTS.LT(&iscpTS) while the other checks deleteTS.IsEmpty() && deleteTS.LT(&iscpTS). This inconsistency may lead to different GC behavior for similar objects. if iscpTables == nil { bm.Add(uint64(i)) continue } if iscpTS, ok := iscpTables[entry.table]; ok { if entry.stats.GetCNCreated() || entry.stats.GetAppendable() { if (!entry.dropTS.IsEmpty() && entry.dropTS.LT(&iscpTS)) || entry.createTS.GT(&iscpTS) { continue } } } bm.Add(uint64(i)) } continue } } if !createTS.LT(ts) || !deleteTS.LT(ts) { continue } if deleteTS.IsEmpty() { logutil.Warn("GC-PANIC-TS-EMPTY", zap.String("name", name), zap.String("createTS", createTS.ToString())) continue } if !logtail.ObjectIsSnapshotRefers( &stats, pitr, &createTS, &deleteTS, snapshots, ) { if iscpTables == nil { bm.Add(uint64(i)) continue } if iscpTS, ok := iscpTables[tableID]; ok { if stats.GetCNCreated() || stats.GetAppendable() { if (deleteTS.IsEmpty() && deleteTS.LT(&iscpTS)) || createTS.GT(&iscpTS) { continue } } }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix empty timestamp handling logic The logic for checking ISCP table timestamps has a potential bug. When entry.dropTS.IsEmpty() is true, the condition !entry.dropTS.IsEmpty() && entry.dropTS.LT(&iscpTS) will always be false, making the entire OR condition depend only on entry.createTS.GT(&iscpTS). This may not handle empty drop timestamps correctly for ISCP filtering. pkg/vm/engine/tae/db/gc/v3/exec_v1.go [362-374] if iscpTables == nil { bm.Add(uint64(i)) continue } if iscpTS, ok := iscpTables[entry.table]; ok { if entry.stats.GetCNCreated() || entry.stats.GetAppendable() { - if (!entry.dropTS.IsEmpty() && entry.dropTS.LT(&iscpTS)) || + if (entry.dropTS.IsEmpty() || entry.dropTS.LT(&iscpTS)) || entry.createTS.GT(&iscpTS) { continue } } } bm.Add(uint64(i)) Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where a live object (with an empty dropTS) would be incorrectly garbage collected, and the proposed fix resolves this issue.	High
	✅ Fix contradictory timestamp condition logic Suggestion Impact: The suggestion identified the logical contradiction and the commit implemented a fix, though with a different approach than suggested. Instead of changing && to ||, the commit negated the first condition to !deleteTS.IsEmpty() code diff: - if (deleteTS.IsEmpty() && deleteTS.LT(&iscpTS)) || + if (!deleteTS.IsEmpty() && deleteTS.LT(&iscpTS)) || The condition deleteTS.IsEmpty() && deleteTS.LT(&iscpTS) is logically incorrect. If deleteTS is empty, it cannot be less than iscpTS at the same time. This creates unreachable code and incorrect filtering behavior for objects with empty delete timestamps. pkg/vm/engine/tae/db/gc/v3/exec_v1.go [391-403] if iscpTables == nil { bm.Add(uint64(i)) continue } if iscpTS, ok := iscpTables[tableID]; ok { if stats.GetCNCreated() || stats.GetAppendable() { - if (deleteTS.IsEmpty() && deleteTS.LT(&iscpTS)) || + if (deleteTS.IsEmpty() || deleteTS.LT(&iscpTS)) || createTS.GT(&iscpTS) { continue } } } bm.Add(uint64(i)) [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a logically impossible condition (deleteTS.IsEmpty() && ...) that results in a bug where live objects could be garbage collected, and the proposed fix is correct.	High
Update