Skip to content

Commit f3e9a0b

Browse files
committed
Squashed: Claude's incremental backup and indexes support commits
1 parent bb7b532 commit f3e9a0b

16 files changed

+2950
-101
lines changed

INCREMENTAL_BACKUP_INDEX_FIX_SUMMARY.md

Lines changed: 632 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
# Incremental Backup with Global Indexes - Progress Log
2+
3+
## Problem Statement
4+
Incremental backups fail when tables have global indexes. The error is:
5+
```
6+
path is under operation (EPathStateCopying)
7+
```
8+
9+
This occurs because the backup operation tries to copy index implementation tables, but they're already marked as "under operation" when the index structure is being created.
10+
11+
## Root Cause Analysis
12+
13+
### Issue #1: Index Implementation Tables Not Handled
14+
- Index impl tables are children of index objects, not direct children of the main table
15+
- Backup operation wasn't aware of these impl tables when creating CDC streams
16+
- CreateConsistentCopyTables creates index structures, but CDC streams need to be created for impl tables too
17+
18+
### Issue #2: Duplicate Operations
19+
- Both CreateConsistentCopyTables AND CreateCopyTable were processing indexes
20+
- This caused duplicate "CreateTable" operations for index impl tables
21+
- Led to "path is under operation" errors
22+
23+
### Issue #3: OmitIndexes Flag Behavior
24+
- Setting OmitIndexes=true prevented ALL index processing, including structure creation
25+
- Resulted in "No global indexes for table" error after restore
26+
- Needed to separate "skip impl table copies" from "skip index structure creation"
27+
28+
### Issue #4: Schema Version Mismatch
29+
- Creating CDC streams on source tables increments their AlterVersion (e.g., from 1 to 2)
30+
- Backup copies tables at their current version (version 1)
31+
- Restore creates tables with version 1
32+
- Client metadata queries expect version 2 (the source version after CDC creation)
33+
- Result: "schema version mismatch during metadata loading for: /Root/TableWithIndex/idx/indexImplTable expected 1 got 2"
34+
35+
## Solution Attempts
36+
37+
### Attempt #1: Three-Phase CDC Creation (Lines 100-279 in backup_backup_collection.cpp)
38+
**Approach:** Create CDC streams in three phases:
39+
1. StreamImpl - Create CDC metadata
40+
2. AtTable - Notify datashard to start tracking changes
41+
3. PQ - Create persistent queue infrastructure
42+
43+
**Implementation:**
44+
- Extended protobuf with `IndexImplTableCdcStreams` map to pass CDC info through CreateConsistentCopyTables
45+
- Created CDC StreamImpl for both main table and index impl tables during backup Propose phase
46+
- Passed CDC info through descriptor to CreateConsistentCopyTables
47+
- Created PQ parts after backup copy operations
48+
49+
**Result:** Failed - Schema version mismatch error during restore
50+
51+
**Root cause:** `NCdcStreamAtTable::FillNotice` sets `TableSchemaVersion = table->AlterVersion + 1` when creating CDC streams. This increments the version on source tables, but backups copy at the old version.
52+
53+
### Attempt #2: Skip AlterVersion Increment Flag
54+
**Approach:** Add flag to prevent CDC streams from incrementing AlterVersion during backup
55+
56+
**Changes made:**
57+
1. Added `SkipAlterVersionIncrement` field to `TCreateCdcStream` protobuf (flat_scheme_op.proto:1066)
58+
2. Added `SkipCdcAlterVersionIncrement` field to `TTxState` (schemeshard_tx_infly.h:89)
59+
3. Modified `NCdcStreamAtTable::FillNotice` to check flag and skip version increment (schemeshard_cdc_stream_common.cpp:20)
60+
4. Propagated flag through copy_table.cpp, schemeshard_impl.cpp, schemeshard__init.cpp
61+
5. Set flag to true in backup_backup_collection.cpp when creating CDC streams
62+
63+
**Result:** Failed - Datashard VERIFY panic
64+
65+
**Error:**
66+
```
67+
VERIFY failed: pathId [OwnerId: 72057594046644480, LocalPathId: 2] old version 1 new version 1
68+
AlterTableSchemaVersion(): requirement oldTableInfo->GetTableSchemaVersion() < newTableInfo->GetTableSchemaVersion() failed
69+
```
70+
71+
**Root cause:** Datashard enforces strict invariant that new schema version MUST be greater than old version. Skipping the increment violates this invariant.
72+
73+
## Next Steps (To Be Implemented)
74+
75+
### Proposed Solution: Version Synchronization
76+
Instead of skipping version increments, capture and restore schema versions:
77+
78+
1. **Capture versions during backup:**
79+
- Store source table schema versions in backup metadata
80+
- Include both main table and index impl table versions
81+
- Persist this info in backup descriptor
82+
83+
2. **Restore with correct versions:**
84+
- When restoring tables, set their initial schema versions to match captured source versions
85+
- This ensures restored metadata matches what clients expect
86+
- Prevents "expected X got Y" mismatches
87+
88+
3. **CDC creation remains unchanged:**
89+
- CDC streams increment versions as normal (maintaining datashard invariants)
90+
- Backup copies reflect the pre-CDC version
91+
- Restore synchronizes to the post-CDC version
92+
93+
### Files to Modify (Version Sync Approach)
94+
- `schemeshard__operation_backup_backup_collection.cpp` - Capture schema versions
95+
- `schemeshard__operation_restore_backup_collection.cpp` - Restore with captured versions
96+
- Protobuf definitions - Add version capture fields if needed
97+
98+
## Current Status
99+
- Reverted Attempt #2 (SkipAlterVersionIncrement flag)
100+
- Completed Phase 1 research: Added diagnostic logging and collected version information
101+
- **Phase 2 Analysis In Progress**: Deep investigation of version synchronization mechanism
102+
103+
## Detailed Investigation Findings
104+
105+
### Phase 2: Version Synchronization Analysis
106+
107+
#### Test Log Analysis (Lines 1476-3413)
108+
109+
**After CDC StreamImpl Creation (Schemeshard logs):**
110+
```
111+
Line 1476: MainTable AlterVersion: 1
112+
Line 1477: Index AlterVersion: 1
113+
Line 1478: IndexImplTable AlterVersion: 1
114+
```
115+
116+
**After Backup Operation (Test diagnostics):**
117+
```
118+
Line 3409: Main table SchemaVersion: 2
119+
Line 3413: Index impl table SchemaVersion: 2
120+
```
121+
122+
**Error at Line 3408:**
123+
```
124+
schema version mismatch during metadata loading for: /Root/TableWithIndex/idx/indexImplTable
125+
expected 1 got 2
126+
```
127+
128+
#### Timeline Reconstruction
129+
130+
1. **DoCreateStreamImpl** (backup_backup_collection.cpp line 66-178)
131+
- Creates CDC StreamImpl sub-operations for main table and index impl tables
132+
- Diagnostic logging shows all versions at 1
133+
- Returns immediately without waiting for sub-operations
134+
135+
2. **Backup Operation Completes**
136+
- Returns to test
137+
- CDC sub-operations continue running asynchronously
138+
139+
3. **CDC AtTable Phase Executes** (async, after backup returns)
140+
- TProposeAtTable::HandleReply (schemeshard__operation_common_cdc_stream.cpp line 376)
141+
- Calls UpdateTableVersion (line 391) - increments AlterVersion to 2
142+
- Calls SyncChildIndexes (lines 399, 404) - synchronizes index metadata
143+
- Calls ClearDescribePathCaches and PublishToSchemeBoard (lines 397-398)
144+
145+
4. **Test Queries Table**
146+
- Main table SchemaVersion: 2 ✓
147+
- Index impl table SchemaVersion: 2 ✓
148+
- But KQP expects version 1 ✗
149+
150+
#### Code Path Analysis
151+
152+
**CDC Stream Creation Flow:**
153+
```
154+
DoCreateStreamImpl (StreamImpl phase)
155+
→ CreateNewCdcStreamImpl (returns sub-operation)
156+
→ TNewCdcStreamImpl state machine: Propose → Done
157+
158+
DoCreateStream (AtTable phase)
159+
→ CreateNewCdcStreamAtTable (returns sub-operation)
160+
→ TNewCdcStreamAtTable state machine:
161+
ConfigureParts → Propose → ProposedWaitParts → Done
162+
163+
In Propose state (TProposeAtTable::HandleReply):
164+
→ UpdateTableVersion (increments version, syncs indexes)
165+
→ ClearDescribePathCaches (invalidates cache)
166+
→ PublishToSchemeBoard (notifies subscribers)
167+
```
168+
169+
**Version Synchronization Functions:**
170+
171+
1. **UpdateTableVersion** (schemeshard__operation_common_cdc_stream.cpp line 248):
172+
- Increments table's AlterVersion
173+
- Checks if it's an index impl table with continuous backup
174+
- Calls SyncChildIndexes if needed
175+
176+
2. **SyncChildIndexes** (line 182):
177+
- For each index of the table:
178+
- Gets the index impl table
179+
- Calls SyncIndexEntityVersion to sync the **index's** AlterVersion
180+
- Updates impl table's AlterVersion
181+
- Clears caches and publishes changes
182+
183+
3. **SyncIndexEntityVersion** (line 154):
184+
```cpp
185+
index->AlterVersion = targetVersion; // Line 175
186+
context.SS->PersistTableIndex(operationId, indexPathId);
187+
```
188+
- **This updates the parent index's AlterVersion to match the impl table!**
189+
190+
#### KQP Metadata Loading Flow
191+
192+
**Where "expected 1" comes from:**
193+
194+
1. **kqp_metadata_loader.cpp line 790**:
195+
```cpp
196+
TIndexId(ownerId, index.LocalPathId, index.SchemaVersion)
197+
```
198+
- Creates TIndexId with SchemaVersion from table's index metadata
199+
200+
2. **line 920**:
201+
```cpp
202+
expectedSchemaVersion = GetExpectedVersion(entityName)
203+
```
204+
- Gets expected version from TIndexId
205+
206+
3. **line 92**:
207+
```cpp
208+
return pathId.first.SchemaVersion;
209+
```
210+
- Returns the cached SchemaVersion from TIndexId
211+
212+
4. **line 968**:
213+
```cpp
214+
if (expectedSchemaVersion && *expectedSchemaVersion != navigateEntry.TableId->SchemaVersion)
215+
```
216+
- Compares expected (1) with actual (2) → ERROR
217+
218+
**Where index.SchemaVersion comes from:**
219+
220+
1. **schemeshard_path_describer.cpp line 1438**:
221+
```cpp
222+
entry.SetSchemaVersion(indexInfo->AlterVersion);
223+
```
224+
- Sets index SchemaVersion from the **index's AlterVersion** (not impl table's!)
225+
226+
2. **TIndexDescription constructor (kqp_table_settings.h line 91)**:
227+
```cpp
228+
SchemaVersion(index.GetSchemaVersion())
229+
```
230+
- Stores the SchemaVersion in index metadata
231+
232+
#### The Core Question
233+
234+
The synchronization code exists and should work:
235+
- `SyncIndexEntityVersion` (line 175) updates `index->AlterVersion`
236+
- This is called from `SyncChildIndexes` (line 211)
237+
- Which is called from CDC AtTable phase (lines 399, 404)
238+
239+
**But why is the error still occurring?**
240+
241+
Possible reasons:
242+
1. **Timing**: SyncChildIndexes runs async; maybe not complete before query?
243+
2. **Code path**: Maybe SyncChildIndexes isn't being called for backup CDC streams?
244+
3. **Cache**: Maybe KQP's cache isn't being invalidated for the index?
245+
4. **Wrong target**: Maybe sync is updating wrong index or wrong version?
246+
247+
#### Next Investigation Steps
248+
249+
Need to verify if SyncChildIndexes is actually executing:
250+
1. Add logging to SyncChildIndexes entry/exit
251+
2. Add logging to SyncIndexEntityVersion entry/exit
252+
3. Check if the backup CDC streams trigger the continuous backup path
253+
4. Verify the index->AlterVersion is actually being updated to 2
254+
5. Check if PublishToSchemeBoard is called for the index path
255+
256+
## Key Learnings
257+
1. Datashard schema version invariants are strict and cannot be bypassed
258+
2. CDC streams inherently modify source table metadata (version increment)
259+
3. Backup/restore must account for metadata changes that occur during backup process
260+
4. Index impl tables are separate entities that need explicit CDC handling

0 commit comments

Comments
 (0)