Author: Jason Gustafson <jason@confluent.io>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#3133 from hachikuji/minor-replica-manager-append-refactor
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Apurva Mehta <apurva@confluent.io>, Jun Rao <junrao@gmail.com>
Closes#3075 from hachikuji/KAFKA-5259-FIXED
Clarify the consumer group command help message around `zookeeper`, `bootstrap-server`, and `new-consumer` options.
Author: Vahid Hashemian <vahidhashemian@us.ibm.com>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#2046 from vahidhashemian/minor/improve_consumer_group_command_doc
Before this patch the consumer would return the cached offsets for partitions in its current assignment. This worked when all the offset commits went through the consumer.
With KIP-98, offsets can be committed transactionally through the producer. This means that relying on cached positions in the consumer returns incorrect information: since commits go through the producer, the cache is never updated.
Hence we need to update the `KafkaConsumer.committed` method to always lookup the server for the last committed offset to ensure it gets the correct information every time.
Author: Apurva Mehta <apurva@confluent.io>
Reviewers: Jason Gustafson, Guozhang Wang
Closes#3119 from apurvam/KAFKA-5273-kafkaconsumer-committed-should-always-hit-server
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jun Rao <junrao@gmail.com>, Onur Karaman <okaraman@linkedin.com>
Closes#2983 from ijuma/kafka-5135-controller-health-metrics-kip-143
This ticket is all about ControllerContext initialization and teardown. The key points are:
1. we should teardown ControllerContext during resignation instead of waiting on election to fix it up. A heapdump shows that the former controller keeps pretty much all of its ControllerContext state laying around.
2. we don't properly teardown/reset ControllerContext.partitionsBeingReassigned. This can cause problems when the former controller becomes re-elected as controller at a later point in time.
Suppose a partition assignment is initially R0. Now suppose a reassignment R1 gets stuck during controller C0 and an admin tries to "undo" R1 (by deleting /admin/partitions_reassigned, deleting /controller, and submitting another reassignment specifying R0). The new controller C1 may succeed with R0. If the controller moves back to C0, it will then reattempt R1 even though that partition reassignment has been cleared from zookeeper prior to shifting the controller back to C0. This results in the actual partition reassignment in zookeeper being unexpectedly changed back to R1.
Author: Onur Karaman <okaraman@linkedin.com>
Reviewers: Jun Rao <junrao@gmail.com>
Closes#3122 from onurkaraman/KAFKA-5310
Two major changes plus one minor change:
0. change stateLock to a read-write lock.
1. Put the check of "isCoordinator" and "coordinatorLoading" together with the return of the metadata, under one read lock block, since otherwise we can get incorrect behavior if there is a change in the metadata cache after the check but before the accessing of the metadata.
2. Grab the read lock right before trying to append to local txn log, and until the local append returns; this is to avoid the scenario that the epoch has actually changed when we are appending to local log (e.g. emigration followed by immigration).
3. only watch on txnId instead of txnId and txnPartitionId in the txn marker purgatory, and disable reaper thread, as we can now safely clear all the delayed operations by traversing the marker queues.
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Jason Gustafson, Jun Rao
Closes#3082 from guozhangwang/K5231-read-write-lock
`TransactionCoordinatorIntegrationTest` is not covering anything that isn't already covered by the more complete `TransactionsTest`
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#3128 from dguy/minor-remove-test
With this patch, offset commits are always materialized according to the order of the commit records in the offsets topic.
Before this patch, transactional offset commits were materialized in transaction order. However, the log cleaner will always preserve the record with the greatest offset. This meant that if there was a mix of offset commits from a consumer and a transactional producer, then it we would switch from transactional order to offset order after cleaning, resulting in an inconsistent state.
Author: Apurva Mehta <apurva@confluent.io>
Reviewers: Guozhang Wang <wangguoz@gmail.com>, Jason Gustafson <jason@confluent.io>
Closes#3108 from apurvam/KAFKA-5247-materialize-committed-offsets-in-offset-order
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#3118 from hachikuji/disallow-transactional-idempotent-downconversion
`shutdownIdleFetcherThreads()` can throw InterruptedException
for example.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3096 from ijuma/kafka-5289-stop-replica-should-not-send-two-responses
We should retry AddPartitionsToTxnRequest and TxnOffsetCommitRequest when receiving an UNKNOWN_TOPIC_OR_PARTITION error.
As described in the JIRA: It turns out that the `UNKNOWN_TOPIC_OR_PARTITION` is returned from the request handler in KafkaAPis for the AddPartitionsToTxn and the TxnOffsetCommitRequest when the broker's metadata doesn't contain one or more partitions in the request. This can happen for instance when the broker is bounced and has not received the cluster metadata yet.
We should retry in these cases, as this is the model followed by the consumer when committing offsets, and by the producer with a ProduceRequest.
Author: Apurva Mehta <apurva@confluent.io>
Reviewers: Guozhang Wang <wangguoz@gmail.com>, Jason Gustafson <jason@confluent.io>
Closes#3094 from apurvam/KAFKA-5269-handle-unknown-topic-partition-in-transaction-manager
Summary:
- add `reconnect.backoff.max.ms` common client configuration parameter
- if `reconnect.backoff.max.ms` > `reconnect.backoff.ms`, apply an exponential backoff policy
- apply +/- 20% random jitter to smooth cluster reconnects
Author: Dana Powers <dana.powers@gmail.com>
Reviewers: Ewen Cheslack-Postava <me@ewencp.org>, Roger Hoover <roger.hoover@gmail.com>, Ismael Juma <ismael@juma.me.uk>
Closes#1523 from dpkp/exp_backoff
- Avoid unnecessary inner methods
- Remove redundant parameter in `sendResponseExemptThrottle`
- Go through `sendResponseExemptThrottle` for produce requests with acks=0
- Tighten how we handle cases where there’s no response
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3087 from ijuma/kafka-apis-improvements
Perform down conversion after throttling to avoid retaining
messages in memory during throttling since this could result
in OOM. Also update bytesOut metrics after throttling.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3068 from rajinisivaram/KAFKA-5250
Use IBM Kerberos module for SASL tests if running on IBM JDK
Developed with edoardocomar
Based on https://github.com/apache/kafka/pull/738 by rajinisivaram
Author: Mickael Maison <mickael.maison@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Rajini Sivaram <rajinisivaram@googlemail.com>, Edoardo Comar <ecomar@uk.ibm.com>
Closes#2878 from mimaison/KAFKA-3070
This is a rebase version of [PR#2973](https://github.com/apache/kafka/pull/2973).
guozhangwang , please review this updated PR.
Author: umesh chaudhary <umesh9794@gmail.com>
Reviewers: Guozhang Wang <wangguoz@gmail.com>, Jason Gustafson <jason@confluent.io>
Closes#3086 from umesh9794/mylocal
Today PartitionStateMachine and ReplicaStateMachine define and assert the
valid state transitions inline for each state. It's cleaner to move the
transition rules into ReplicaState/PartitionState and do the assertion at
the top of the handleStateChange.
Author: Onur Karaman <okaraman@linkedin.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3071 from onurkaraman/KAFKA-5258
mjsax dguy guozhangwang Could you please review the changes.
Author: Bharat Viswanadham <bharatv@us.ibm.com>
Reviewers: Matthias J. Sax, Damian Guy, Guozhang Wang
Closes#3073 from bharatviswa504/KAFKA-5210
This PR implements a new partition assignment strategy called "sticky", and it's purpose is to balance partitions across consumers in a way that minimizes moving partitions around, or, in other words, preserves existing partition assignments as much as possible.
This patch is co-authored with rajinisivaram and edoardocomar.
Author: Vahid Hashemian <vahidhashemian@us.ibm.com>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#1020 from vahidhashemian/KAFKA-2273
Includes server-side code, protocol and AdminClient.
Author: Colin P. Mccabe <cmccabe@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#2941 from cmccabe/KAFKA-3266
…ffsetForLeaderEpoch request
Author: Jun Rao <junrao@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Ben Stopford <benstopford@gmail.com>
Closes#3074 from junrao/kafka-5036
Author: Apurva Mehta <apurva@confluent.io>
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#2994 from apurvam/KAFKA-5188-exactly-once-integration-tests
Author: Jorge Quilcate <quilcate.jorge@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
Closes#2624 from jeqo/feature/rewind-consumer-group-offset
Reduce rebalance and session timeouts for join requests to trigger throttling in the request quota test.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Damian Guy <damian.guy@gmail.com>
Closes#3057 from rajinisivaram/KAFKA-5182-quotatest
Mirrormaker was updated to default to the new consumer in 3db752a565
Old consumer calls the param `auto.commit.enable`, new consumer calls it `enable.auto.commit`. So I updated the docstring to use the new consumer name for the param.
Author: Jeff Widman <jeff@jeffwidman.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#2393 from jeffwidman/patch-1
Addressed the below review comment from #PR #2998 from mjsax
I am wondering if it would be better, to "embed" the dry-run into the actual code and branch on each place. Otherwise, if things get changed, we could easily introduce bugs (ie, dry run show something different than what the actual reset code does.
We could introduce methods like mabyeSeekToBeginning() that either does the seek or only prints to stdout. This would ensure that the main logic is used to "feed" into dry-run and we don't have code duplication.
WDYT?
Author: Bharat Viswanadham <bharatv@us.ibm.com>
Reviewers: Matthias J. Sax, Damian Guy, Eno Thereska, Guozhang Wang
Closes#3005 from bharatviswa504/KAFKA-5166
Add ACL checks for Transactional APIs
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Apurva Mehta <apurva@confluent.io>, Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
Closes#2979 from dguy/kafka-5129
The transient failure came from the controller processing the preferred replica leader election
before the restarted broker (the preferred replica leader) has joined isr, causing preferred
replica leader election to fail and for the final zookeeper state validation to fail.
Author: Onur Karaman <okaraman@linkedin.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3067 from onurkaraman/KAFKA-5175
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Apurva Mehta <apurva@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Closes#3064 from hachikuji/KAFKA-5252
Also disable a couple of tests that were passing incorrectly until KAFKA-3096 is fixed.
The bug was for the following case:
`leader.isDefined && oldLeaderOpt.isEmpty && newLeaderOpt.isDefined && newLeaderOpt.get != leader.get`
We would consider it a successful election even though the new leader was not the expected leader.
I also changed the result type as we never return `None` (we throw an exception instead).
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jun Rao <junrao@gmail.com>
Closes#3031 from ijuma/fix-wait-until-leader-is-elected
This removes the need to force object initialisation via hacks to register
the relevant Yammer metrics during start-up.
It also works around issues caused by tests that delete JVM-wide singleton
metrics (like `MetricsDuringTopicCreationDeletionTest`). Without this
change, they would never be registered again. After this change, they will
be registered again during KafkaServer start-up.
It would be even better not to rely on JVM side singleton metrics (like we do
for Kafka Metrics), but that's a bigger change that should be considered
separately.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3059 from ijuma/kafka-5244-broker-static-stats-and-controller-stats-as-classes
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Guozhang Wang <wangguoz@gmail.com>, Apurva Mehta <apurva@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Closes#3058 from hachikuji/KAFKA-5248
Producer id is used instead.
Also refactored TransactionLog schema code to follow
our naming convention and to have better structure.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Guozhang Wang <wangguoz@gmail.com>, Jason Gustafson <jason@confluent.io>
Closes#3041 from ijuma/eliminate-pid-terminology
* Fix flakiness of `testBrokerTopicMetricsUnregisteredAfterDeletingTopic` by not
consuming messages. Filed KAFKA-5238 to track the issue that metrics for a deleted
topic may be re-created if there are fetch requests in the purgatory.
* Check the log size in `testBrokerTopicMetricsBytesInOut` before attempting to read
the `replicationBytesIn` metric. This helps understand where things have gone wrong
if if the metric has not increased (i.e. if it was an issue replicating or with the metric).
* Only remove the replication bytes in/out if the metrics are defined. This should not
affect the behaviour due to the tags, but it makes the code clearer. We've seen some
cases in Jenkins when the metric does not exist and it's still unclear how that can
happen.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3042 from ijuma/more-informative-assertion-for-flaky-metrics-test
This issue would only be triggered if a broker was restarted while
deletion was still taking place.
Included a few minor improvements to that method and its tests.
Author: Jaikiran Pai <jaikiran.pai@gmail.com>
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3050 from jaikiran/KAFKA-5232-trunk
During the 0.11.0.0 cycle, a Java version of the class
was introduced so that Streams could use it. Given that
it includes the bulk of the functionality of the Scala
version of the class, it makes sense to consolidate them.
While doing this, I noticed that one of the tests for
the Java class (`shouldThrowOnInvalidTopicNames`) was
broken as it only checked if the first topic name in
the list was invalid.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#3046 from ijuma/consolidate-topic-classes
`testBrokerTopicMetricsUnregisteredAfterDeletingTopic` seemed completely broken,
as it was creating a topic but producing/consuming to another one.
Authored with mpburg
Author: Mickael Maison <mickael.maison@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3034 from mimaison/Fix-testBrokerTopicMetricsBytesInOut
Jason Gustafson
Vahid Hashemian
Apurva Mehta
Ismael Juma
Onur Karaman
Guozhang Wang
Damian Guy
Matthias J. Sax
Dana Powers
Rajini Sivaram
Mickael Maison
umesh chaudhary
Bharat Viswanadham
Colin P. Mccabe
Jun Rao
Jorge Quilcate
Jeff Widman
Jaikiran Pai