Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] When the kafka producer send errors, it may not propagate that error if transactions are off #1173

Open
3 tasks done
baroquebobcat opened this issue Dec 2, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@baroquebobcat
Copy link
Contributor

baroquebobcat commented Dec 2, 2024

Describe the bug

#968 made transactions optional. But there is code underneath it that assumes it happens in a transaction and discards a future from the producer client:

https://github.com/slackhq/astra/blob/master/astra/src/main/java/com/slack/astra/bulkIngestApi/BulkIngestKafkaProducer.java#L336-L339

        // we intentionally suppress FutureReturnValueIgnored here in errorprone - this is because
        // we wrap this in a transaction, which is responsible for flushing all of the pending
        // messages
        kafkaProducer.send(producerRecord);

If the producer has an error when the transactions are disabled, it won't get surfaced in logs or http responses. Instead, it will cause requests to the bulk ingest endpoint to silently time out.

Requirements (place an x in each of the [ ])**

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

To Reproduce

Steps to reproduce the behavior:
One option would be to misconfigure kafka with transactions disabled. Eg, a dataset with a partition listed that doesn't exist, or bad auth or a unreasonably short timeout.
But you can also see it by changing the transaction system property setting in the BulkIngestKafkaProducerTest to false

-    System.setProperty("astra.bulkIngest.useKafkaTransactions", "true");
+    System.setProperty("astra.bulkIngest.useKafkaTransactions", "false");

Expected behavior

A clear and concise description of what you expected to happen.

Errors are logged

Screenshots

If applicable, add screenshots to help explain your problem.

Reproducible in:

Astra version: HEAD

JVM version:

OS version(s):

Additional context

Add any other context about the problem here.

Related: #618 - The misconfiguration we had that triggered this is related to this other issue, but it is separate in that the logs having no information instead of "a significant amount of confusing messages".

@baroquebobcat baroquebobcat added the bug Something isn't working label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant