- Remove DelayedCreatePartitions to reduce code duplication
- Avoid unnecessary ZK calls (call it once per request instead
of once per topic, if possible)
- Simplify code
- A few minor clean-ups
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Tom Bentley <tbentley@redhat.com>, Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3930 from ijuma/kafka-5856-admin-client-creation-partitions
1. Raise AuthenticationException for authentication failures in admin client
2. Handle AuthenticationException as a fatal error for transactional producer
3. Add comments to authentication exceptions
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Vahid Hashemian <vahidhashemian@us.ibm.com>, Ismael Juma <ismael@juma.me.uk>
Closes#3928 from rajinisivaram/KAFKA-5947-auth-failure
The contribution is my original work and I license the work to the project under the project's open source license.
This patch adds AdminClient.createPartitions() and the network protocol is
uses. The broker-side algorithm is as follows:
1. KafkaApis makes some initial checks on the request, then delegates to the
new AdminManager.createPartitions() method.
2. AdminManager.createPartitions() performs some validation then delegates to
AdminUtils.addPartitions().
Aside: I felt it was safer to add the extra validation in
AdminManager.createPartitions() than in AdminUtils.addPartitions() since the
latter is used on other code paths which might fail differently with the
introduction of extra checks.
3. AdminUtils.addPartitions() does its own checks and adds the partitions.
4. AdminManager then uses the existing topic purgatory to wait for the
PartitionInfo available from the metadata cache to become consistent with
the new total number of partitions.
The messages of exceptions thrown in AdminUtils affecting this new API have
been made consistent with initial capital letter and terminating period.
A few have been reworded for clarity.
Author: Tom Bentley <tbentley@redhat.com>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, Ismael Juma <ismael@juma.me.uk>
Closes#3870 from tombentley/KAFKA-5856-AdminClient.createPartitions
This PR implements the client-side of KIP-152, by modifying `KafkaConsumer`, `KafkaProducer`, and `ConsumerGroupCommand` to throw a non-retriable exception when SASL authentication fails.
This PR is co-authored with rajinisivaram.
Author: Vahid Hashemian <vahidhashemian@us.ibm.com>
Reviewers: Jason Gustafson <jason@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>, Ismael Juma <ismael@juma.me.uk>, tedyu <yuzhihong@gmail.com>
Closes#3832 from vahidhashemian/KAFKA-5854
Author: Matthias J. Sax <matthias@confluent.io>
Reviewers: Damian Guy <damian.guy@gmail.com>, Jason Gustafson <jason@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Closes#3884 from mjsax/minor-fixed-discoverd-via-exception-handling-investigation
Before this change, the test always failed on my MacBook Pro.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3901 from ijuma/fix-unresolvable-address-in-zookeeper-test
This refactor achieves the following:
1. Breaks up the increasingly unmanageable `Protocol` class and moves schemas closer to their actual usage.
2. Removes the need for redundant field identifiers maintained separately in `Protocol` and the respective request/response objects.
3. Provides a better mechanism for sharing common fields between different schemas (e.g. topics, partitions, error codes, etc.).
4. Adds convenience helpers to `Struct` for common patterns (such as setting a field only if it exists).
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3813 from hachikuji/protocol-schema-refactor
Use logIdent to achieve this.
Also fixed an issue where we were logging about replicas going offline with
an empty set of replicas (i.e. no replicas had gone offline so no need to log).
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#3869 from ijuma/improve-state-change-log
The string representation of TopicPartition was changed to be
{topic}-{partitition} consistently in the following commit:
f6f56a645b
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Damian Guy <damian.guy@gmail.com>
Closes#3890 from ijuma/fix-replica-verification-test
- improve stderr output for better debugging
Author: Matthias J. Sax <matthias@confluent.io>
Reviewers: Bill Bejeck <bill@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Closes#3859 from mjsax/kafka-5893-reset-integration-test
Here we introduce client and broker changes to support multiple inflight requests while still guaranteeing idempotence. Two major problems to be solved:
1. Sequence number management on the client when there are request failures. When a batch fails, future inflight batches will also fail with `OutOfOrderSequenceException`. This must be handled on the client with intelligent sequence reassignment. We must also deal with the fatal failure of some batch: the future batches must get different sequence numbers when the come back.
2. On the broker, when we have multiple inflights, we can get duplicates of multiple old batches. With this patch, we retain the record metadata for 5 older batches.
Author: Apurva Mehta <apurva@confluent.io>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#3743 from apurvam/KAFKA-5494-increase-max-in-flight-for-idempotent-producer
It adds complexity for no benefit since we don't use
it anywhere.
Also removed a few unused imports, variables and
default parameters.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Rajini Sivaram <rajinisivaram@googlemail.com>, Jason Gustafson <jason@confluent.io>
Closes#3856 from ijuma/remove-security-protocol-trace
- EasyMock 3.5 supports Java 9.
- Fixed issues in `testFailedSendRetryLogic` and
`testCreateConnectorAlreadyExists` exposed by new EasyMock
version. The former was passing `anyObject` to
`andReturn`, which doesn't make sense. This was leaving
behind a global `any` matcher, which caused a few issues in
the new version. Fixing this meant that the correlation ids had
to be updated to actually match. The latter was missing a
couple of expectations that the previous version of EasyMock
didn't catch.
- Removed unnecessary PowerMock dependency from 3 tests.
- Disabled remaining PowerMock tests when running with Java 9
until https://github.com/powermock/powermock/issues/783 is
in a release.
- Once we merge this PR, we can enable tests in the Java 9 builds
in Jenkins.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3845 from ijuma/kafka-4501-easymock-powermock-java-9
Changes:
1. When an exception is encountered in any of the methods in `Processor` while processing a channel, log the exception and close the connection. Continue to process other channels.
2. Fixes KAFKA-5790: SocketServer.processNewResponses should not skip a response if exception is thrown.
3. For `IllegalStateException` and `IOException` in `poll()`, don't close the `Selector`. Log the exception and continue.
4. Close channel on any failed send in `Selector`.
5. When closing channel fails or is closed, leave channel state as-is, indicating the state in which the channel was moved to closing.
6. Add tests for various failure scenarios.
7. Fix timing issue in `SocketServerTest.testConnectionIdReuse` by waiting for new connections to be processed by the server.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Jun Rao <junrao@gmail.com>, Ismael Juma <ismael@juma.me.uk>
Closes#3548 from rajinisivaram/KAFKA-5607
As described in the JIRA ticket, when `listeners=PLAINTEXT://0.0.0.0:9092`
(note the 0.0.0.0 "bind all interfaces" IP address) and
`advertised.listeners` is not specified it defaults to `listeners`,
but it makes no sense to advertise 0.0.0.0 as it's not a routable IP
address.
This patch checks for a 0.0.0.0 host in `advertised.listeners`
(whether via default or not) and fails with a meaningful error if it's
found.
This contribution is my original work and I license the work to the
project under the project's open source license.
Author: Tom Bentley <tbentley@redhat.com>
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Ismael Juma <ismael@juma.me.uk>
Closes#3382 from tombentley/advertised.listeners
It's implemented such that there is no overhead if request logging is
disabled.
Also:
- Reduce metrics computation duplication in `updateRequestMetrics`
- Change a couple of log calls to use string interpolation instead of `format`
- Fix a few compiler warnings related to unused imports and unused default
arguments.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Roger Hoover <roger.hoover@gmail.com>, Jason Gustafson <jason@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3801 from ijuma/log-response-in-request-log
- changed the interface & implementations
- updated tests to use the new method where applicable
Author: Attila Kreiner <attila@kreiner.hu>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
Closes#3669 from attilakreiner/KAFKA-5726
Author: Dong Lin <lindong28@gmail.com>
Reviewers: Jun Rao <junrao@gmail.com>, Jiangjie Qin <becket.qin@gmail.com>, Colin P. Mccabe <cmccabe@confluent.io>
Closes#3621 from lindong28/KAFKA-5694
Author: Dong Lin <lindong28@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jiangjie Qin<becket.qin@gmail.com>
Closes#3709 from lindong28/KAFKA-5759
* When a call is aborted, that should count as a "try" in the failure log message.
* FailureInjectingTimeoutProcessorFactory should fail the first request it is asked about.
* testCallTimeouts should expect the first request it makes to fail because of the timeout we injected.
* FailureInjectingTimeoutProcessorFactory should track how many failures it has injected, and the test should verify that one has been injected.
Author: Colin P. Mccabe <cmccabe@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3731 from cmccabe/KAFKA-5720
Remove the previous TODO to remove the clean shutdown file with some of the discussion from https://github.com/apache/kafka/pull/2104.
Author: Holden Karau <holden@us.ibm.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3742 from holdenk/KAFKA-4380-document-clean-shutdown-file
This patch contains a few small improvements to make request/response handling more consistent. Primarily it consolidates request/response serialization logic so that `SaslServerAuthenticator` and `KafkaApis` follow the same path. It also reduces the amount of custom logic needed to handle unsupported versions of the ApiVersions requests.
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3673 from hachikuji/consolidate-response-handling
Two requests sent together may not always trigger a staged receive since the requests may not be received in a single poll and the channel is muted when receives are complete. Hence attempt to stage multiple times until a receive is staged to make the test more stable.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Apurva Mehta <apurva@confluent.io>
Closes#3712 from rajinisivaram/MINOR-connectionidreuse-test
timeIndex and txnIndex were not being updated previously.
Author: Manikumar Reddy <manikumar.reddy@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3700 from omkreddy/KAFKA-5752
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Apurva Mehta <apurva@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Closes#3688 from hachikuji/KAFKA-5747
It actually refers to the `delete` cleanup policy.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#3634 from ijuma/fix-misleading-compact-log-config
This patch ensures that the consumer groupId and clientId are available in all log messages which makes debugging much easier when a single application has multiple consumer instances. To make this easier, I've added a new `LogContext` object which builds a log prefix similar to the broker-side `kafka.utils.Logging` mixin. Additionally this patch changes the log level for a couple minor cases:
- Consumer wakeup events are now logged at DEBUG instead of TRACE
- Heartbeat enabling/disabling is now logged at DEBUG instead of TRACE
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Ismael Juma <ismael@juma.me.uk>
Closes#3676 from hachikuji/log-consumer-wakeups
When there are broker delays that cause a response to take longer
than `connections.max.idle.ms`, connections may be closed by the
broker (as well as by the client) before the response is processed.
If the port is reused, broker may send the outstanding response to
a new connection with the reused port. The new connection will end
up with correlation id mismatch, requiring process restart. This
is also a security exposure since clients receive response
intended for the wrong connection.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3530 from rajinisivaram/KAFKA-5595
These handlers were previously used on the broker to handle uncaught exceptions, but now the broker users the new Java request objects exclusively.
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3646 from hachikuji/remove-old-request-error-handlers
when "auto.offset.reset" property is specified on the command line but overridden by the code during startup. Currently the ConsoleConsumer silently overrides that setting, which can create confusing behavior.
Author: Soenke Liebau <soenke.liebau@opencore.com>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#3566 from soenkeliebau/KAFKA-5629
Notable updates:
1. Gradle 4.1 includes a number of performance and
CLI improvements as well as initial Java 9 support.
2. Scala 2.12.3 has substantial compilation time
improvements.
3. lz4-java 1.4 allows us to remove a workaround in
KafkaLZ4BlockInputStream (not done in this PR).
4. snappy-java 1.1.4 improved performance of compression (5%)
and decompression (20%). There was a slight increase in the
compressed size in one of our tests.
Not updated:
1. PowerMock due to a couple of regressions. I investigated one of them
and filed https://github.com/powermock/powermock/issues/828.
2. Jackson, which will be done via #3631.
3. Rocksdb, which will be done via #3519.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jason Gustafson <jason@confluent.io>
Closes#3619 from ijuma/update-deps-for-1.0.0
Separate `batch.size`, `message-size` and `compression-code` from PerfConfig to a newly-created ProducerPerfConfig in order to hide them in ConsumerPerf tool.
Author: huxihx <huxi_2b@hotmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3613 from huxihx/KAFKA-2360
This field doesn't seem to be used and the value for
`AwaitingSync` seems to be wrong (it seems like it
should have been `2` instead of `5`).
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jason Gustafson <jason@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Closes#3572 from ijuma/remove-unused-group-state-field
Added deprecation and warning message when the --new-consumer
option is used with the ConsumerGroupCommand and
the ConsumerPerformance tools.
Author: Paolo Patierno <ppatierno@live.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3555 from ppatierno/kafka-5619
Ismael Juma
Rajini Sivaram
Tom Bentley
Apurva Mehta
Vahid Hashemian
huxihx
Matthias J. Sax
Jason Gustafson
Jeff Widman
Andrey Dyachkov
Attila Kreiner
Dong Lin
Colin P. Mccabe
Holden Karau
Manikumar Reddy
Soenke Liebau
Balint Molnar
Paolo Patierno