Skip to content

fix: Wait for Kafka lookup startup catch-up - #19858

Open
sakshichitnis27 wants to merge 1 commit into
apache:masterfrom
sakshichitnis27:Issue/fix-kafka-lookup-startup-catchup-19575
Open

fix: Wait for Kafka lookup startup catch-up#19858
sakshichitnis27 wants to merge 1 commit into
apache:masterfrom
sakshichitnis27:Issue/fix-kafka-lookup-startup-catchup-19575

Conversation

@sakshichitnis27

Copy link
Copy Markdown

Fixes #19575

Description

KafkaLookupExtractorFactory previously reported a lookup as started immediately after the consumer's first poll(), even when the consumer had not finished reading the existing topic data. This allowed newly started brokers to serve queries using a partially populated lookup.

This change:

  • Captures the end offsets after the consumer receives its initial partition assignment.
  • Processes polled records before checking catch-up progress.
  • Releases the startup latch only after every assigned partition reaches its captured end offset.
  • Refreshes the captured offsets if the partition assignment changes during startup.
  • Stops checking offsets after startup completes.

The existing connectTimeout continues to bound how long start() waits. A value of 0 retains the existing do-not-wait behavior.

A regression test verifies that start() remains blocked while the consumer is behind and succeeds after all initial records have been applied to the lookup.

Testing

mvn test -pl extensions-core/kafka-extraction-namespace -am \
  -Dtest=org.apache.druid.query.lookup.KafkaLookupExtractorFactoryTest \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -Dweb.console.skip=true \
  -T1C

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity Findings
P0 0
P1 0
P2 1
P3 0
Total 1

Reviewed 2 of 2 changed files. Found one startup-timeout cleanup race where the Kafka worker may keep writing after its cache is disposed.


This is an automated review by Codex GPT-5.6-Sol

doubleEventCount.incrementAndGet();
LOG.trace("Placed key[%s] val[%s]", key, message);
}
if (startingReads.getCount() > 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Wait for the catch-up worker before disposing its cache

When a finite connectTimeout expires while this new catch-up loop is processing a backlog, start() calls future.cancel(true) and immediately cacheHandler.close(). Cancellation only interrupts the worker; the record loop has no interrupt check, so it can continue put/remove calls after close. With the off-heap cache manager, close deletes the underlying MapDB map, racing those writes against deletion. Coordinate worker termination, or move cache disposal into the worker's completion path, before closing the cache.

public void testStartStop()
{
Consumer<String, String> kafkaConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST);
final MockConsumer<String, String> kafkaConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST);
@Test
public void testStartWaitsForInitialEndOffsets() throws Exception
{
final MockConsumer<String, String> kafkaConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST);
public void testStartStartStopStop()
{
Consumer<String, String> kafkaConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST);
final MockConsumer<String, String> kafkaConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants