fix: change query id generation to work with planned commands #4149
Conversation
This patch changes up how we generate query IDs to play nice with planned commands. Before this change, statements would get the current offset as their query id. However planned commands get their query IDs before being enqueued, so they should really get the _next_ expected offset as their ID. This patch changes up the id generation to work this way. The next ID is set _after_ statemetns/plans are executed, and is set to the next expected offset.
|
@rodesai Just to clarify the behavior to expect, If I have (beginning of command topic) CS, CT, CSAS, CT, CT, CTAS, the ID's would be |
|
@rodesai This PR got me thinking, with the Transactional Protocol in place, isn't it possible that we create a Plan with a QueryId (which uses up the current identifier in the generator), but the server fails to enqueue the plan to the command topic. Wouldn't that mean the server wouldn't be able to send a CSAS/CTAS to the command topic unless it executes another CSAS/CTAS sent there by another server? |
When generating plans, the distributor uses a snapshot engine and generator, which doesn't consume an ID from the actual engine/generator. |
...rc/test/java/io/confluent/ksql/rest/server/computation/InteractiveStatementExecutorTest.java
Outdated
Show resolved
Hide resolved
…utation/InteractiveStatementExecutorTest.java Co-Authored-By: Victoria Xia <[email protected]>
Description
This patch changes up how we generate query IDs to play nice with
planned commands. Before this change, statements would get the current
offset as their query id. However planned commands get their query IDs
before being enqueued, so they should really get the next expected
offset as their ID. This patch changes up the id generation to work
this way. The next ID is set after statements/plans are executed,
and is set to the next expected offset.