Skip to content

Commit 74d7ad2

Browse files
oleksandr-ncoleksandr-nc
committed
fix(config): paginate the legacy backfill reads and clear the migration gate flag once no legacy table remains
Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
1 parent 3067622 commit 74d7ad2

2 files changed

Lines changed: 82 additions & 50 deletions

File tree

‎lib/Migration/Version035000Date20260529120000.php‎

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Closure;
1313
use OCP\Config\IUserConfig;
1414
use OCP\DB\ISchemaWrapper;
15+
use OCP\DB\QueryBuilder\IQueryBuilder;
1516
use OCP\IAppConfig;
1617
use OCP\IDBConnection;
1718
use OCP\Migration\IOutput;
@@ -38,6 +39,7 @@
3839
* rows are copied as-is and keep round-tripping through the listener. Do not sniff ciphertext.
3940
* - Idempotent: a key already present in the target is skipped (standard storage wins), so a
4041
* re-run after a transient failure resumes cleanly. All ExApp values are stored lazy.
42+
* - Rows are read in keyset-paginated batches so a large `preferences_ex` cannot OOM `occ upgrade`.
4143
*/
4244
class Version035000Date20260529120000 extends SimpleMigrationStep {
4345

@@ -48,6 +50,9 @@ class Version035000Date20260529120000 extends SimpleMigrationStep {
4850
*/
4951
public const FAILED_FLAG = 'migration_035000_backfill_failed';
5052

53+
/** Read the legacy tables in keyset-paginated batches of this many rows to bound memory usage. */
54+
private const BATCH_SIZE = 1000;
55+
5156
public function __construct(
5257
private IDBConnection $connection,
5358
private IAppConfig $appConfig,
@@ -68,7 +73,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
6873
$failed += $this->migratePreferences($output);
6974
}
7075

71-
// Persist the outcome so the table-drop migration (PR #2) can gate on it. Stored lazy under
76+
// Persist the outcome so the table-drop migration can gate on it. Stored lazy under
7277
// AppAPI's own app id; a value > 0 means some rows are still only in the legacy tables.
7378
$this->appConfig->setValueString('app_api', self::FAILED_FLAG, (string)$failed, lazy: true);
7479
if ($failed > 0) {
@@ -81,27 +86,31 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
8186
private function migrateAppConfig(IOutput $output): int {
8287
$migrated = $skipped = $failed = 0;
8388

84-
foreach ($this->fetchRows('appconfig_ex', ['id', 'appid', 'configkey', 'configvalue', 'sensitive']) as $row) {
85-
$appId = (string)$row['appid'];
86-
$configKey = (string)$row['configkey'];
87-
$sensitive = (int)($row['sensitive'] ?? 0) === 1;
88-
89-
try {
90-
if ($this->appConfig->hasKey($appId, $configKey, null)) {
91-
$skipped++;
92-
continue;
93-
}
94-
$value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive);
95-
if ($value === null) {
96-
$output->warning(sprintf('Config migration: failed to decrypt sensitive value for app %s key %s (row id=%d) — skipping', $appId, $configKey, (int)$row['id']));
89+
$lastId = 0;
90+
while (($rows = $this->fetchBatch('appconfig_ex', ['appid', 'configkey', 'configvalue', 'sensitive'], $lastId)) !== []) {
91+
foreach ($rows as $row) {
92+
$lastId = (int)$row['id'];
93+
$appId = (string)$row['appid'];
94+
$configKey = (string)$row['configkey'];
95+
$sensitive = (int)($row['sensitive'] ?? 0) === 1;
96+
97+
try {
98+
if ($this->appConfig->hasKey($appId, $configKey, null)) {
99+
$skipped++;
100+
continue;
101+
}
102+
$value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive);
103+
if ($value === null) {
104+
$output->warning(sprintf('Config migration: failed to decrypt sensitive value for app %s key %s (row id=%d) — skipping', $appId, $configKey, $lastId));
105+
$failed++;
106+
continue;
107+
}
108+
$this->appConfig->setValueString($appId, $configKey, $value, lazy: true, sensitive: $sensitive);
109+
$migrated++;
110+
} catch (Throwable $e) {
111+
$output->warning(sprintf('Config migration: failed to migrate app %s key %s (row id=%d): %s — skipping', $appId, $configKey, $lastId, $e->getMessage()));
97112
$failed++;
98-
continue;
99113
}
100-
$this->appConfig->setValueString($appId, $configKey, $value, lazy: true, sensitive: $sensitive);
101-
$migrated++;
102-
} catch (Throwable $e) {
103-
$output->warning(sprintf('Config migration: failed to migrate app %s key %s (row id=%d): %s — skipping', $appId, $configKey, (int)$row['id'], $e->getMessage()));
104-
$failed++;
105114
}
106115
}
107116

@@ -112,32 +121,36 @@ private function migrateAppConfig(IOutput $output): int {
112121
private function migratePreferences(IOutput $output): int {
113122
$migrated = $skipped = $failed = 0;
114123

115-
foreach ($this->fetchRows('preferences_ex', ['id', 'userid', 'appid', 'configkey', 'configvalue', 'sensitive']) as $row) {
116-
$userId = (string)$row['userid'];
117-
$appId = (string)$row['appid'];
118-
$configKey = (string)$row['configkey'];
119-
$sensitive = (int)($row['sensitive'] ?? 0) === 1;
120-
121-
try {
122-
if ($this->userConfig->hasKey($userId, $appId, $configKey, null)) {
123-
$skipped++;
124-
continue;
125-
}
126-
$value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive);
127-
if ($value === null) {
128-
$output->warning(sprintf('Preferences migration: failed to decrypt sensitive value for user %s app %s key %s (row id=%d) — skipping', $userId, $appId, $configKey, (int)$row['id']));
124+
$lastId = 0;
125+
while (($rows = $this->fetchBatch('preferences_ex', ['userid', 'appid', 'configkey', 'configvalue', 'sensitive'], $lastId)) !== []) {
126+
foreach ($rows as $row) {
127+
$lastId = (int)$row['id'];
128+
$userId = (string)$row['userid'];
129+
$appId = (string)$row['appid'];
130+
$configKey = (string)$row['configkey'];
131+
$sensitive = (int)($row['sensitive'] ?? 0) === 1;
132+
133+
try {
134+
if ($this->userConfig->hasKey($userId, $appId, $configKey, null)) {
135+
$skipped++;
136+
continue;
137+
}
138+
$value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive);
139+
if ($value === null) {
140+
$output->warning(sprintf('Preferences migration: failed to decrypt sensitive value for user %s app %s key %s (row id=%d) — skipping', $userId, $appId, $configKey, $lastId));
141+
$failed++;
142+
continue;
143+
}
144+
$this->userConfig->setValueString(
145+
$userId, $appId, $configKey, $value,
146+
lazy: true,
147+
flags: $sensitive ? IUserConfig::FLAG_SENSITIVE : 0,
148+
);
149+
$migrated++;
150+
} catch (Throwable $e) {
151+
$output->warning(sprintf('Preferences migration: failed to migrate user %s app %s key %s (row id=%d): %s — skipping', $userId, $appId, $configKey, $lastId, $e->getMessage()));
129152
$failed++;
130-
continue;
131153
}
132-
$this->userConfig->setValueString(
133-
$userId, $appId, $configKey, $value,
134-
lazy: true,
135-
flags: $sensitive ? IUserConfig::FLAG_SENSITIVE : 0,
136-
);
137-
$migrated++;
138-
} catch (Throwable $e) {
139-
$output->warning(sprintf('Preferences migration: failed to migrate user %s app %s key %s (row id=%d): %s — skipping', $userId, $appId, $configKey, (int)$row['id'], $e->getMessage()));
140-
$failed++;
141154
}
142155
}
143156

@@ -162,15 +175,21 @@ private function decryptLegacyValue(string $rawValue, bool $sensitive): ?string
162175
}
163176

164177
/**
165-
* Materialize the full rowset up front: iterating a forward-only cursor while writing config
166-
* (which touches the DB on the same connection) is undefined across drivers.
178+
* Fetch up to {@see BATCH_SIZE} rows whose id is greater than `$afterId`, ordered by id, then
179+
* close the cursor before the caller issues any writes. Keyset pagination keeps memory bounded on
180+
* large tables and avoids holding a forward-only cursor open while writing config on the same
181+
* connection. The backfill never modifies the legacy tables, so paging over them is stable.
167182
*
168183
* @param string[] $columns
169184
* @return list<array<string,mixed>>
170185
*/
171-
private function fetchRows(string $table, array $columns): array {
186+
private function fetchBatch(string $table, array $columns, int $afterId): array {
172187
$qb = $this->connection->getQueryBuilder();
173-
$qb->select(...$columns)->from($table);
188+
$qb->select('id', ...$columns)
189+
->from($table)
190+
->where($qb->expr()->gt('id', $qb->createNamedParameter($afterId, IQueryBuilder::PARAM_INT)))
191+
->orderBy('id', 'ASC')
192+
->setMaxResults(self::BATCH_SIZE);
174193
$cursor = $qb->executeQuery();
175194
$rows = $cursor->fetchAll();
176195
$cursor->closeCursor();

‎lib/Migration/Version035000Date20260529130000.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,26 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
5858
}
5959

6060
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
61-
// The outcome flag only existed to gate this drop; remove it once the tables are gone.
62-
if ($this->dropped && $this->appConfig->hasKey('app_api', Version035000Date20260529120000::FAILED_FLAG, null)) {
61+
// The outcome flag only existed to gate this drop. Once no legacy table remains it is dead
62+
// bookkeeping, so clear it whether this run dropped them or they were already gone. While a
63+
// table still exists (e.g. the drop was skipped due to backfill failures) the flag is kept so
64+
// a later re-run still sees the failure count.
65+
if ($this->appConfig->hasKey('app_api', Version035000Date20260529120000::FAILED_FLAG, null)
66+
&& !$this->anyLegacyTableExists()) {
6367
$this->appConfig->deleteKey('app_api', Version035000Date20260529120000::FAILED_FLAG);
6468
}
6569
return null;
6670
}
6771

72+
private function anyLegacyTableExists(): bool {
73+
foreach (self::LEGACY_TABLES as $table) {
74+
if ($this->connection->tableExists($table)) {
75+
return true;
76+
}
77+
}
78+
return false;
79+
}
80+
6881
/**
6982
* Decide whether it is safe to drop the legacy tables. The backfill records its outcome in the
7083
* app config key `app_api / <FAILED_FLAG>`:

0 commit comments

Comments
 (0)