The test_throttled_reassignment test fails because the consumer that is used to validate reassignment does not start on time to consume all messages. This does not seem like an issue with the throttling of the reassignment, since increasing the timeout allowed the test to pass multiple consecutive runs locally.
This test seemed to rely on the default JmxTool for the console consumer that was removed in this commit: 179d0d7
The console consumer would check to see if it had partitions assigned to it before beginning to consume. Although the test occasionally failed with the JmxTool, it began to fail much more after the removal.
Error messages of failures followed the below format with varying numbers of missed messages. They are the first messages by the producer.
535 acked message did not make it to the Consumer. They are: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19...plus 515 more. Total Acked: 192792, Total Consumed: 192259. We validated that the first 535 of these missing messages correctly made it into Kafka's data files. This suggests they were lost on their way to the consumer.
In the scope of the test, this error suggests that the test is falling into the race condition described in produce_consume_validate.py, which has the timeout to prevent the consumer from missing initial messages.
This can serve as a temporary fix until the logic of consumer startup is addressed further.
Reviewers: Jason Gustafson <jason@confluent.io>, Bill Bejeck <bbejeck@gmail.com>
9.4.25 renamed closeOutput to completeOutput
(c5acf96506),
which is a method used by recent Jersey versions including the
latest (2.30.1). An example of the error:
> java.lang.NoSuchMethodError: org.eclipse.jetty.server.Response.closeOutput()V
> at org.glassfish.jersey.jetty.JettyHttpContainer$ResponseWriter.commit(JettyHttpContainer.java:326)
The request still completes and hence why no test fails. We should think about how
to improve the testing for this kind of problem, but I want to get the fix in before
2.5 RC0.
Credit to @rigelbm for finding this.
Reviewers: Ewen Cheslack-Postava <me@ewencp.org>, Andrew Choi <a24choi@edu.uwaterloo.ca>
Confirmed the test passes after this (minor) change.
Reviewers: Jason Gustafson <jason@confluent.io>, Boyang Chen <boyang@confluent.io>, Ismael Juma <ismael@juma.me.uk>
For handleRevocation, it is possible that previous onAssignment callback has cleaned up the stream tasks, which means no corresponding task could be found for given partitions. We should not throw here as this is expected behavior.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
We only initialize topology when transiting from restoring -> running.
Also tighten some unit tests for this fix:
a. restoring -> suspended should just write checkpoint file without committing.
b. suspended -> restoring should not need any inner updates.
c. restoring -> running should always try to fetch committed offsets, and forward timeout exceptions.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, John Roesler <john@confluent.io>, Boyang Chen <boyang@confluent.io>
Adds a couple of extra checks to the test-output-capturing logic in our gradle build.
Previously, we were seeing a lot of error logs while attempting to write output for a
test whose output file hadn't been initialized.
Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>
This PR adds new lock is used to prevent the follower replica from being updated while ReplicaAlterDirThread is executing maybeReplaceCurrentWithFutureReplica() to replace follower replica with the future replica.
Now doAppendRecordsToFollowerOrFutureReplica() doesn't need to hold the lock on leaderIsrUpdateLock for local replica updation and ongoing log appends on the follower will not delay the makeFollower() call.
**Benchmark results for Partition.makeFollower() **
Old:
```
Benchmark Mode Cnt Score Error Units
PartitionMakeFollowerBenchmark.testMakeFollower avgt 15 2046.967 ? 22.842 ns/op
```
New:
```
Benchmark Mode Cnt Score Error Units
PartitionMakeFollowerBenchmark.testMakeFollower avgt 15 1278.525 ? 5.354 ns/op
```
Author: Manikumar Reddy <manikumar.reddy@gmail.com>
Reviewers: Jun Rao <junrao@gmail.com>
Closes#8153 from omkreddy/KAFKA-9594-LAISR
This bug reproduces through the trunk stream test, the producer was closed unexpectedly when EOS is not turned on.
Will work on adding unit test to guard this logic.
Reviewers: Guozhang Wang <wangguoz@gmail.com>
This PR avoids generating unnecessary TopicChange events during the topic validation. It does so by adding a registerWatch field in the GetChildrenRequest request. This allows to not register the watch when topics are queried from the topic validation logic.
Reviewers: Ismael Juma <ismael@juma.me.uk>, Stanislav Kozlovski <stanislav_kozlovski@outlook.com>, Jason Gustafson <jason@confluent.io>, Jun Rao <junrao@gmail.com>
Right now the task assignor just blindly assigns N standby tasks per active task (where N = num.standbys) and attempts to distribute them evenly across all instances/threads. But only standby tasks that are stateful, and whose stores are changelog-enabled, will ever actually be created.
This can result in a less-balanced assignment, and should be cleaned up in particular before implementing KIP-441 to remove the noise of ghost standbys.
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Reworded the ssl part of the security documentation to fix various issues (mainly as noted by this jira, the problem that SAN extension values are not copied to certificates) and add some recommendations.
Reviewers: Mickael Maison <mickael.maison@gmail.com>
This patch improves the authorizer integration tests in the following ways:
1. We use a separate principal for inter-broker communications. This ensures that ACLs set in the test cases do not interfere with inter-broker communication. We had two test cases (`testCreateTopicAuthorizationWithClusterCreate` and `testAuthorizationWithTopicExisting`) which depend on topic creation and were timing out because of inter-broker metadata propagation failures. The timeouts were treated as successfully satisfying the expectation of authorization. So the tests passed, but not because of the intended reason.
2. Previously `GroupAuthorizerIntegrationTest` was inheriting _all_ of the tests from `AuthorizerIntegrationTest`. This seemed like overkill since the ACL evaluation logic is essentially the same.
Totally this should take about 5-10 minutes off the total build time and make the authorizer integration tests a little more resilient to problems with inter-broker communication.
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, Ismael Juma <ismael@juma.me.uk>
The "offset deletion" and "group rebalance" should not be recorded by the same sensor since they are totally different.
The code is introduced by #7276.
Reviewers: David Jacot <djacot@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
The rebalance exception withholding is no longer necessary as we have better mechanism for catching and wrapping these exceptions. Throw them directly should be fine and simplify our current error handling.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
With a short timeout, a call in KafkaAdminClient may timeout and the client might disconnect. Currently this can be exposed to the user as either a TimeoutException or a DisconnectException. To be consistent, rather than exposing the underlying retriable error, we handle both cases with a TimeoutException.
Reviewers: Boyang Chen <boyang@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Author: Andras Katona <akatona@cloudera.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Manikumar Reddy <manikumar.reddy@gmail.com>
Closes#7953 from akatona84/kafkarunclass-auto-scala-version
The SaslClientAuthenticator incorrectly negotiates supported SaslHandshakeRequest version and uses the maximum version supported by the broker whether or not the client supports it.
This bug was exposed by a recent version bump in 0a2569e2b9.
This PR rolls back the recent SaslHandshake[Request,Response] bump, fixes the version negotiation, and adds a test to prevent anyone from accidentally bumping the version without a workaround such as a new ApiKey. The existing key will be difficult to support for clients < 2.5 due to the incorrect negotiation.
Reviewers: Ron Dagostino <rdagostino@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>, Colin P. McCabe <cmccabe@apache.org>, Jason Gustafson <jason@confluent.io>
Upfront refactoring for KIP-447.
Introduces `StreamsProducer` that allows to share a producer over multiple tasks and track the TX status.
Reviewers: Boyang Chen <boyang@confluent.io>, Guozhang Wang <guozhang@confluent.io>
Update the site documentation to include the endpoints introduced with KIP-558 and a short paragraph on how this feature is used in Connect.
Author: Konstantine Karantasis <konstantine@confluent.io>
Reviewers: Toby Drake <tobydrake7@gmail.com>, Ewen Cheslack-Postava <ewen@confluent.io>
Closes#8148 from kkonstantine/kip-558-docs
This is a minor fix of a regression introduced in the refactoring PR: in current trunk standbyTask#commitNeeded always return false, which would cause standby tasks to never be committed until closed. To go back to the old behavior we would return true when new data has been applied and offsets being updated.
Reviewers: Boyang Chen <boyang@confluent.io>, John Roesler <john@confluent.io>
If a completed fetch has an error code signifying a _corrupt message_, throw a `KafkaException` that notes the fetch offset and the topic-partition.
Reviewers: Jason Gustafson <jason@confluent.io>
Author: Manikumar Reddy <manikumar.reddy@gmail.com>
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ismael Juma <ismael@juma.me.uk>
Closes#8072 from omkreddy/release-script
This PR is the counterpart of apache/kafka-site#253.
cc/ omkreddy
Author: Lee Dongjin <dongjin@apache.org>
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
Closes#8149 from dongjinleekr/feature/KAFKA-9586
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
Author: Ron Dagostino <rdagostino@confluent.io>
Reviewers: Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
Closes#8139 from rondagostino/KAFKA-9575
1. Removed task field from TaskMigrated; the only caller that encodes a task id from StreamTask actually do not throw so we only log it. To handle it on StreamThread we just always enforce rebalance (and we would call onPartitionsLost to remove all tasks as dirty).
2. Added TaskCorruptedException with a set of task-ids. The first scenario of this is the restoreConsumer.poll which throws InvalidOffset indicating that the logs are truncated / compacted. To handle it on StreamThread we first close the corresponding tasks as dirty (if EOS is enabled we would also wipe out the state stores), and then revive them into the CREATED state.
3. Also fixed a bug while investigating KAFKA-9572: when suspending / closing a restoring task we should not commit the new offsets but only updating the checkpoint file.
4. Re-enabled the unit test.
This fixes two issues which together caused the soak to crash/some test to fail occasionally.
What happened was: In the main StreamThread loop we initialized a new task in TaskManager#checkForCompletedRestoration which includes registering, but not initializing, its changelogs. We then complete the loop and call poll, which resulted in a rebalance that revoked the newly-initialized task. In TaskManager#handleAssignment we then closed the task cleanly and go to remove the changelogs from the StoreChangelogReader only to get an IllegalStateException because the changelog partitions were not in the restore consumer's assignment (due to being uninitialized).
This by itself should^ be a recoverable error, as we catch exceptions here and retry closing the task as unclean. Of course the task actually was successfully closed (clean) so we now get an unexpected exception Illegal state CLOSED while closing active task
The fix(es) I'd propose are:
1. Keep the restore consumer's assignment in sync with the registered changelogs, ie the set ChangelogReader#changelogs but pause them until they are initialized edit: since the consumer does still perform some actions (gg fetches) on paused partitions, we should avoid adding uninitialized changelogs to the restore consumer's assignment. Instead, we should just skip them when removing.
2. Move the StoreChangelogReader#remove call to before the task.closeClean so that the task is only marked as closed if everything was successful. We should do so regardless, as we should (attempt to) remove the changelogs even if the clean close failed and we must do unclean.
Reviewers: John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
The current EOS example mixes fatal and non-fatal error handling. This patch fixes this problem and simplifies the example.
Reviewers: Jason Gustafson <jason@confluent.io>
Fixes a bug where KStream#transformValues would forward null values from the provided ValueTransform#transform operation.
A test was added for verification KStreamTransformValuesTest#shouldEmitNoRecordIfTransformReturnsNull. A parallel test for non-key ValueTransformer was not added, as they share the same code path.
Reviewers: Bruno Cadonna <bruno@confluent.io>, Bill Bejeck <bbejeck@gmail.com>
There is a race condition with the backoff sleep in the test case and setting the next allowed send time in the AdminClient. To fix it, we allow the test case to do the backoff sleep multiple times if needed.
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
This PR is to fix the retry logic for `getListOffsetsCalls`. Previously, if there were partitions with errors, it would only pass in the current call object to retry after a metadata refresh. However, if there's a leader change, the call object never gets updated with the correct leader node to query. This PR fixes this by making another call to `getListOffsetsCalls` with only the error topic partitions as the next calls to be made after the metadata refresh. In addition there is an additional test to test the scenario where a leader change occurs.
Reviewers: Jason Gustafson <jason@confluent.io>
Previously, checkpointed offsets for a log were only updated if the log was chosen for cleaning once the cleaning job completes. This caused issues in cases where logs with invalid checkpointed offsets would repeatedly emit warnings if the log with an invalid cleaning checkpoint wasn't chosen for cleaning.
Proposed fix is to update the checkpointed offset for logs with invalid checkpoints regardless of whether it gets chosen for cleaning.
Reviewers: Anna Povzner <anna@confluent.io>, Jun Rao <junrao@gmail.com>
Matthew Wong
Ismael Juma
Bruno Cadonna
Boyang Chen
Agam Brahma
Guozhang Wang
Chris Egerton
John Roesler
Manikumar Reddy
Chia-Ping Tsai
David Jacot
A. Sophie Blee-Goldman
high.lee
Bill Bejeck
Tom Bentley
Sönke Liebau
Ron Dagostino
Jason Gustafson
Matthias J. Sax
Andras Katona
Lucas Bradstreet
Konstantine Karantasis
Lee Dongjin
Mickael Maison
zshuo
Gunnar Morling
Stanislav Kozlovski
Michael Viamari
Sanjana Kaundinya
David Mao