Replaces the static `RequestMetrics` object with a class so that metrics
are created and removed during broker startup and shutdown to avoid metrics
tests being affected by metrics left behind by previous tests.
Also reinstates `kafka.api.MetricsTest` which was failing frequently earlier
due to tests removing the static request metrics.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3991 from rajinisivaram/KAFKA-5968
It should be the number of records instead of the
number of batches.
A few additional clean-ups:
- Minor renames
- Removed unused variable
- Some test fixes
- Ignore a flaky test
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, tedyu <yuzhihong@gmail.com>
Closes#3989 from ijuma/kafka-5746-health-metrics-follow-up
Adds new metrics to support health checks:
1. Error rates for each request type, per-error code
2. Request size and temporary memory size
3. Message conversion rate and time
4. Successful and failed authentication rates
5. ZooKeeper latency and status
6. Client version
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3705 from rajinisivaram/KAFKA-5746-new-metrics
It now records the correct size for `Send` and does more appropriate
logging if the connection is being closed or if no response is being
sent.
Author: huxihx <huxi_2b@hotmail.com>
Reviewers: Denis Bolshakov, Ismael Juma <ismael@juma.me.uk>
Closes#3961 from huxihx/KAFKA-5976
This depends on sun.misc.Signal and sun.misc.SignalHandler, which may be
removed in future releases. But along with sun.misc.Unsafe, these classes
are available in Java 9 (see JEP 260), so they are safe to use for now.
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3668 from rajinisivaram/KAFKA-5679
Author: Jason Gustafson <jason@confluent.io>
Reviewers: Apurva Mehta <apurva@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Closes#3944 from hachikuji/KAFKA-5960
Author: Mickael Maison <mickael.maison@gmail.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
Closes#3036 from mimaison/KAFKA-3356
Offset and partition are not converted from String to
long and int correctly.
Running the command line with --from-file option causes
the following exception:
java.lang.ClassCastException: java.lang.String cannot be
cast to java.lang.Integer
Reason: asInstanceOf used for the conversion.
Also, unit test is using --to-earliest and --from-file
together when executing the test. This is executing
--to-earliest option only and ignoring --from-file
option. Since the preparation part is also using
--to-earliest to create the file, this unit test
passes without testing --from-file option. Fixed
the unit test too.
Author: Erkan Unal <eunal@cisco.com>
Reviewers: Ismael Juma <ismael@juma.me.uk>
Closes#3938 from eu657/eu657-patch-1
As mentioned in MappedByteBuffers' class documentation, its
implementation was inspired by Lucene's MMapDirectory:
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/6.6.1/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L315
Without this change, unmapping fails with the following message:
> java.lang.IllegalAccessError: class kafka.log.AbstractIndex (in unnamed module 0x45103d6b) cannot access class jdk.internal.ref.Cleaner (in module java.base) because module java.base does not export jdk.internal.ref to unnamed module 0x45103d6b
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3879 from ijuma/kafka-5915-unmap-mapped-buffers-java-9
These methods help users check for cases in which this metadata was not returned by the broker (e.g. in the case of acks=0 or a duplicate error when idempotence is enabled).
Author: Apurva Mehta <apurva@confluent.io>
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
Closes#3878 from apurvam/KAFKA-5913-add-record-metadata-not-available-exception
Developed with edoardocomar
Author: Mickael Maison <mickael.maison@gmail.com>
Reviewers: Edoardo Comar <ecomar@uk.ibm.com>, Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#3906 from mimaison/KAFKA-5735
- 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:
f6f56a645bb1c5ec6810c024ba517e43bf77056c
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
Rajini Sivaram
Ismael Juma
huxihx
Jason Gustafson
Manikumar Reddy
Dong Lin
Mickael Maison
Bill Bejeck
Erkan Unal
Apurva Mehta
Tom Bentley
Vahid Hashemian
Matthias J. Sax
Jeff Widman
Andrey Dyachkov
Attila Kreiner
Colin P. Mccabe
Holden Karau