Skip to content

fix(ilp): prevent writes going to the wrong place after a table rename#6654

Merged
bluestreak01 merged 13 commits intomasterfrom
nw_ilp_rename
Jan 16, 2026
Merged

fix(ilp): prevent writes going to the wrong place after a table rename#6654
bluestreak01 merged 13 commits intomasterfrom
nw_ilp_rename

Conversation

@nwoolmer
Copy link
Copy Markdown
Contributor

The bug:

  • Send data to QuestDB via ILP to table 'A'.
  • Hold connection open.
  • Rename table 'A' to 'B'.
  • Create new table 'A'.
  • Send data to QuestDB via ILP to table 'A'.

... actually the data goes to 'B'.

References to the table were cached in the state of each HTTP Connection.

Now there is a global version counter for renames. If this counter bumps, the processing will halt, flush the cache, rollback the partially written WAL, and then return an error to the user for retry. The aim is to preserve the atomicity of the request.

@nwoolmer nwoolmer added Bug Incorrect or unexpected behavior ILP Issues or changes relating to Influx Line Protocol labels Jan 15, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 15, 2026

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This PR introduces table rename detection and generation tracking for ILP caches. A new tableRenameGeneration counter in CairoEngine increments on table rename, which LineHttpTudCache monitors to detect stale state. Upon detection, HTTP 503 (Service Unavailable) is returned to signal clients to retry with fresh cache state.

Changes

Cohort / File(s) Summary
Rename Generation Infrastructure
core/src/main/java/io/questdb/cairo/CairoEngine.java, core/src/main/java/io/questdb/cutlass/http/HttpResponseSink.java
Added tableRenameGeneration AtomicLong counter with public getter to CairoEngine; incremented after rename enqueues view recompilation. Added HTTP 503 constant and status mapping to HttpResponseSink.
ILP Cache Rename Detection
core/src/main/java/io/questdb/cutlass/http/processors/LineHttpTudCache.java
Tracks lastKnownRenameGeneration in cache; detects generation mismatches in getTableUpdateDetails() and clear(); throws new singleton TableRenameException on mismatch. Added try/catch guards during resource rollback and reset to prevent exceptions from blocking cleanup.
HTTP Request Processing
core/src/main/java/io/questdb/cutlass/http/processors/LineHttpProcessorState.java
Extended appendMeasurement() to throw TableRenameException; added handleTableRenameError() handler returning new TABLE_SCHEMA_CHANGED status (503); added exception catch in processLocalBuffer().
Test Coverage
core/src/test/java/io/questdb/test/cutlass/http/line/LineHttpSenderTest.java
Added three end-to-end test scenarios: rename-and-recreate with Sender retry detection, large batch rejection on rename with retry path, and concurrent rename during batch processing with synchronization barriers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Suggested reviewers

  • bluestreak01
  • ideoma
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: preventing writes to the wrong table after a table rename by implementing cache invalidation.
Description check ✅ Passed The description clearly explains the bug scenario, root cause, and the solution involving a global version counter for renames.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nwoolmer
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 15, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@glasstiger
Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 26 / 26 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/cutlass/line/tcp/TableUpdateDetails.java 4 4 100.00%
🔵 io/questdb/cutlass/http/processors/LineHttpTudCache.java 6 6 100.00%
🔵 io/questdb/cutlass/http/HttpResponseSink.java 1 1 100.00%
🔵 io/questdb/cairo/wal/seq/TableSequencerAPI.java 2 2 100.00%
🔵 io/questdb/cairo/pool/WalWriterPool.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/LineHttpProcessorState.java 9 9 100.00%
🔵 io/questdb/cairo/wal/WalWriter.java 3 3 100.00%

@bluestreak01 bluestreak01 merged commit f859544 into master Jan 16, 2026
43 checks passed
@bluestreak01 bluestreak01 deleted the nw_ilp_rename branch January 16, 2026 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Incorrect or unexpected behavior ILP Issues or changes relating to Influx Line Protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants