Commit 93bbb41
committed
[fix](cloud)(restore) fix broken schema during restore of lsc=false tables
Problem
-------
In cloud mode, when a snapshot of a table created with
`light_schema_change = false` is restored, the schema KV persisted under
`meta_schema_key({instance_id, index_id, schema_version})` ends up with
every column carrying `unique_id = -1`, because the schema is first
written by `create_tablet` and at that point the unique_ids are not yet
assigned. `commit_restore_job` then receives the correct schema (with
valid `unique_id >= 0`) in each rowset meta from the backup, but the
existing `put_schema_kv` is a no-op when the key already exists, so the
broken schema leaks through and subsequent reads fail with errors such
as `column reader is nullptr` or `different type between schema and
column reader`.
Fix
---
Introduce `put_schema_kv_on_restore()` in the cloud MetaService. It
reads the existing schema value, detects the broken-schema signature
(`column(0).unique_id() == -1` or an unparseable value), range-removes
all chunks of the stale schema, and writes the correct one. To avoid
replacing a bad schema with another bad one, it also refuses to write
a schema that is itself broken (empty columns or `column(0).unique_id
== -1`) and only logs a warning in that case.
In `MetaServiceImpl::commit_restore_job`, replace both existing
`put_schema_kv` call sites with `put_schema_kv_on_restore`, guarded by
an in-RPC `std::set<std::string>` so the same
`(index_id, schema_version)` pair does not issue redundant FDB
reads/writes when the restore spans many rowsets. Four counters
(`rs_meta_schema_put_cnt/skip_cnt`,
`tablet_meta_schema_put_cnt/skip_cnt`) are logged at the end of the
RPC to make the behaviour observable in production.
`put_versioned_schema_kv()` is intentionally NOT wrapped by the dedup
set: it targets a different key space (`versioned::meta_schema_key`)
and is already skip-if-exists inside the function.
Tests
-----
`cloud/test/meta_service_test.cpp` adds 5 unit tests covering every
branch of `put_schema_kv_on_restore`:
- `PutWhenKeyNotExist` — first-write path
- `NoopWhenExistingSchemaIsGood` — skip-if-healthy path
- `OverwriteWhenExistingIsBroken` — the fix itself
- `DefensiveSkipWhenIncomingHasEmptyColumns` — defensive guard
- `DefensiveSkipWhenIncomingHasUidNegativeOne` — defensive guard1 parent 34b5c6c commit 93bbb41
4 files changed
Lines changed: 419 additions & 6 deletions
File tree
- cloud
- src/meta-service
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
1764 | 1765 | | |
1765 | 1766 | | |
1766 | 1767 | | |
| 1768 | + | |
| 1769 | + | |
| 1770 | + | |
| 1771 | + | |
| 1772 | + | |
| 1773 | + | |
| 1774 | + | |
| 1775 | + | |
| 1776 | + | |
1767 | 1777 | | |
1768 | 1778 | | |
1769 | 1779 | | |
| |||
1791 | 1801 | | |
1792 | 1802 | | |
1793 | 1803 | | |
1794 | | - | |
1795 | | - | |
1796 | | - | |
| 1804 | + | |
| 1805 | + | |
| 1806 | + | |
| 1807 | + | |
| 1808 | + | |
| 1809 | + | |
| 1810 | + | |
| 1811 | + | |
| 1812 | + | |
| 1813 | + | |
1797 | 1814 | | |
1798 | 1815 | | |
1799 | 1816 | | |
| |||
2052 | 2069 | | |
2053 | 2070 | | |
2054 | 2071 | | |
2055 | | - | |
2056 | | - | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
2057 | 2080 | | |
2058 | 2081 | | |
2059 | 2082 | | |
| |||
2161 | 2184 | | |
2162 | 2185 | | |
2163 | 2186 | | |
2164 | | - | |
| 2187 | + | |
| 2188 | + | |
| 2189 | + | |
| 2190 | + | |
| 2191 | + | |
2165 | 2192 | | |
2166 | 2193 | | |
2167 | 2194 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
133 | 134 | | |
134 | 135 | | |
135 | 136 | | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
136 | 188 | | |
137 | 189 | | |
138 | 190 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
30 | 39 | | |
31 | 40 | | |
32 | 41 | | |
| |||
0 commit comments