Skip to content

Commit d25e8b8

Browse files
fix: replace empty catch with logged best-effort cleanup; fix doc comment
- Replace `catch { }` with `catch (Exception cleanupEx) when (cleanupEx is not OperationCanceledException)` + Console.Error.WriteLine so cleanup failures are visible without masking the original exception - Correct method summary: swap uses two SQL RENAMEs (live→old, staging→live) plus DROP TABLE statements, not "three SQL RENAMEs" Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 65071fc commit d25e8b8

1 file changed

Lines changed: 15 additions & 7 deletions

File tree

EssentialCSharp.Chat.Shared/Services/EmbeddingService.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@ public async Task<ReadOnlyMemory<float>> GenerateEmbeddingAsync(string text, Can
4343
/// 1. Create a staging collection ({collectionName}_staging).
4444
/// 2. Embed all chunks in batches of <see cref="EmbeddingBatchSize"/> (Azure OpenAI limit).
4545
/// 3. Batch-upsert all chunks into staging.
46-
/// 4. Atomically swap staging → live via three SQL RENAMEs in a single transaction.
47-
/// PostgreSQL ALTER TABLE acquires AccessExclusiveLock automatically; no explicit
48-
/// LOCK TABLE is needed. The transaction ensures no reader sees an intermediate state.
49-
/// 5. Drop the old live backup table.
46+
/// 4. Atomically swap tables in a single transaction using two SQL RENAME operations
47+
/// (live → old, staging → live). PostgreSQL ALTER TABLE acquires
48+
/// AccessExclusiveLock automatically; no explicit LOCK TABLE is needed. The
49+
/// transaction ensures no reader sees an intermediate state.
50+
/// 5. Drop the old live backup table with DROP TABLE.
5051
///
5152
/// If an error occurs before the swap, only the staging table is affected — the live
5253
/// collection is untouched.
@@ -102,14 +103,21 @@ public async Task GenerateBookContentEmbeddingsAndUploadToVectorStore(
102103
{
103104
// Best-effort cleanup: drop the partially-populated staging table so the
104105
// next run starts clean. Do not let this secondary failure mask the original.
105-
try { await staging.EnsureCollectionDeletedAsync(cancellationToken); } catch { }
106+
try
107+
{
108+
await staging.EnsureCollectionDeletedAsync(cancellationToken);
109+
}
110+
catch (Exception cleanupEx) when (cleanupEx is not OperationCanceledException)
111+
{
112+
Console.Error.WriteLine($"Warning: failed to clean up staging collection '{stagingName}' after upsert failure: {cleanupEx.Message}");
113+
}
106114
throw;
107115
}
108116

109117
// ── Step 4: Atomic swap — staging → live ──────────────────────────────────────
110-
// Three ALTER TABLE RENAME statements in one transaction.
118+
// Two ALTER TABLE RENAME operations in one transaction (live → old, staging → live).
111119
// Each RENAME auto-acquires AccessExclusiveLock on its table; the transaction
112-
// guarantees all three renames are visible atomically to other sessions.
120+
// guarantees both renames are visible atomically to other sessions.
113121
await using var conn = await dataSource.OpenConnectionAsync(cancellationToken);
114122
await using var tx = await conn.BeginTransactionAsync(cancellationToken);
115123

0 commit comments

Comments
 (0)