This patch adds missing flush logic to `KafkaRaftClient`. The initial flushing behavior is simplistic. We guarantee that the leader will not replicate above the last flushed offset and we guarantee that the follower will not fetch data above its own flush point. More sophisticated flush behavior is proposed in KAFKA-10526.
We have also extended the simulation test so that it covers flush behavior. When a node is shutdown, all unflushed data is lost. We were able to confirm that the monotonic high watermark invariant fails without the added `flush` calls.
This patch also piggybacks a fix to the `TestRaftServer` implementation. The initial check-in contained a bug which caused `RequestChannel` to fail sending responses because the disabled APIs did not have metrics registered. As a result of this, it is impossible to elect leaders.
Reviewers: Guozhang Wang <wangguoz@gmail.com>
In LeaderEpochFileCacheTest.scala, code is identical for `shouldNotResetEpochHistoryHeadIfUndefinedPassed` and `shouldNotResetEpochHistoryTailIfUndefinedPassed`. Seems `truncateFromStart` should be invoked in `shouldNotResetEpochHistoryHeadIfUndefinedPassed` instead of `truncateFromEnd`.
In KIP-455, we changed the behavior of the reassignment tool so that the `--additional` flag is required in order to use the command to alter the throttle. This patch improves the documentation to make this clearer and adds some integration tests to validate the behavior.
This patch also contains a few minor code quality improvements:
- Factor out a helper `calculateCurrentMoveMap` from `calculateMoveMap` to compute the current move map, which makes the logic easier to follow
- Rename `calculateMoveMap` to `calculateProposedMoveMap` to make intention clearer
- Split `modifyBrokerThrottles` into two methods `modifyLogDirThrottle` and `modifyInterBrokerThrottle`
- Move logic to compute leader and follower throttles into a new method `modifyReassignmentThrottle`, which takes it out of the execution path when log dir throttles are changed
- Minor stylistic improvements such as replacing `.map.flatten` with `.flatMap`
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, Colin P. McCabe <cmccabe@apache.org>, Ismael Juma <ismael@juma.me.uk>
Previously, we would throw a confusing error, "the store has migrated,"
when users ask for a store that is not in the topology at all, or when the
type of the store doesn't match the QueryableStoreType parameter.
Adds an up-front check that the requested store is registered and also
a better error message when the QueryableStoreType parameter
doesn't match the store's type.
Reviewers: Guozhang Wang <guozhang@apache.org>
https://issues.apache.org/jira/browse/KAFKA-10584
In Scala, we prefer sealed traits over Enumeration since the former gives you exhaustiveness checking. With Scala Enumeration, you don't get a warning if you add a new value that is not handled in a given pattern match.
This PR avoids a performance issue with DeleteRecords when a partition directory contains high numbers of files. Previously, DeleteRecords would iterate the partition directory searching for producer state snapshot files. With this change, the iteration is removed in favor of keeping a 1:1 mapping between producer state snapshot file and segment file. A segment files corresponding producer state snapshot file is now deleted when the segment file is deleted.
Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk>, Jun Rao <junrao@gmail.com>
This PR adds missing broker ACLs required to create topics and SCRAM credentials when ACLs are enabled for a system test. This PR also adds support for using PLAINTEXT as the inter broker security protocol when using SCRAM from the client in a system test with a secured cluster-- without this it would always be necessary to set both the inter-broker and client mechanisms to a SCRAM mechanism. Also contains some refactoring to make assumptions clearer.
Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
While this is not technically part of KIP-629, I believe this makes our codebase more inclusive as well.
cc gwenshap
Author: Xavier Léauté <xvrl@apache.org>
Reviewers: Gwen Shapira
Closes#9398 from xvrl/neutral-term
This replaces code and comment occurrences as described in the KIP
Author: Xavier Léauté <xvrl@apache.org>
Reviewers: Gwen Shapira, Mickael Maison
Closes#9366 from xvrl/kafka-10571
Add additional warning logs and improve existing log messages for `FileNotFoundException` and if /tmp is used as state directory.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Matthias J. Sax <matthias@confluent.io>
StreamThreadStateStoreProvider excessive loop over calling internalTopologyBuilder.topicGroups(), which is synchronized, thus causing significant performance degradation to the caller, especially when store has many partitions.
Reviewers: John Roesler <vvcephei@apache.org>, Guozhang Wang <wangguoz@gmail.com>
In this PR, I have addressed the review comments from @chia7712 in #9001 which were provided after #9001 was merged. The changes are made mainly to KafkaAdminClient:
Improve error message in updateFeatures api when feature name is empty.
Propagate top-level error message in updateFeatures api.
Add an empty-parameter variety for describeFeatures api.
Minor documentation updates to @param and @return to make these resemble other apis.
Reviewers: Chia-Ping Tsai chia7712@gmail.com, Jun Rao junrao@gmail.com
* all wrapping stores should pass StateStoreContext init through to the same
method on the wrapped store and not translate it to ProcessorContext init
* base-level stores should handle StateStoreContext init so that callers passing
a non-InternalProcessorContext implementation will be able to initialize the store
* extra tests are added to verify the desired behavior
Reviewers: Guozhang Wang <guozhang@apache.org>
Avoid continuous repeated logging by not trying to clean empty task directories, which are longer fully deleted during internal cleanup as of https://issues.apache.org/jira/browse/KAFKA-6647.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Summary:
In this PR, I have implemented the write path of the feature versioning system (KIP-584). Here is a summary of what's in this PR:
New APIs in org.apache.kafka.clients.admin.Admin interface, and their client and server implementations. These APIs can be used to describe features and update finalized features. These APIs are: Admin#describeFeatures and Admin#updateFeatures.
The write path is provided by the Admin#updateFeatures API. The corresponding server-side implementation is provided in KafkaApis and KafkaController classes. This can be a good place to start the code review.
The write path is supplemented by Admin#describeFeatures client API. This does not translate 1:1 to a server-side API. Instead, under the hood the API makes an explicit ApiVersionsRequest to the Broker to fetch the supported and finalized features.
Implemented a suite of integration tests in UpdateFeaturesTest.scala that thoroughly exercises the various cases in the write path.
Other changes:
The data type of the FinalizedFeaturesEpoch field in ApiVersionsResponse has been modified from int32 to int64. This change is to conform with the latest changes to the KIP explained in the voting thread.
Along the way, the class SupportedFeatures has been renamed to be called BrokerFeatures, and, it now holds both supported features as well as default minimum version levels.
For the purpose of testing, both the BrokerFeatures and FinalizedFeatureCache classes have been changed to be no longer singleton in implementation. Instead, these are now instantiated once and maintained in KafkaServer. The singleton instances are passed around to various classes, as needed.
Reviewers: Boyang Chen <boyang@confluent.io>, Jun Rao <junrao@gmail.com>
For now, Kafka system tests use python2 which is outdated and not supported.
This PR upgrades python to the third version.
Reviewers: Ivan Daschinskiy, Mickael Maison <mickael.maison@gmail.com>, Magnus Edenhill <magnus@edenhill.se>, Guozhang Wang <wangguoz@gmail.com>
This field is leftover from the early days of the KIP when it covered reassignment. The API is not exposed yet, so there is no harm updating the first version.
Reviewers: Ismael Juma <ismael@juma.me.uk>
Adds support for SSL key and trust stores to be specified in PEM format either as files or directly as configuration values.
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
One of the invariants that the raft replication protocol relies on is that each record is uniquely identified by leader epoch and offset. This can be violated if a leader remains elected with the same epoch between restarts since unflushed data could be lost.
Reviewers: Guozhang Wang <wangguoz@gmail.com>
* Extract the mock RocksDBConfigSetter into a separate class.
* De-dup unit tests covering RocksDBConfigSetter.
Reviewers: Boyang Chen <boyang@confluent.io>
`BlockingConnectorTest` was incorrectly running as a unit test.
Categorize this test correctly as integration test by adding the appropriate annotation
Reviewer: Randall Hauch <rhauch@gmail.com>
The `org.apache.kafka.connect.data.Values#parse` method parses integers, which are larger than `Long.MAX_VALUE` as `double` with `Schema.FLOAT64_SCHEMA`.
That means we are losing precision for these larger integers.
For example:
`SchemaAndValue schemaAndValue = Values.parseString("9223372036854775808");`
returns:
`SchemaAndValue{schema=Schema{FLOAT64}, value=9.223372036854776E18}`
Also, this method parses values that can be parsed as `FLOAT32` to `FLOAT64`.
This PR changes parsing logic, to use `FLOAT32`/`FLOAT64` for numbers that don't have fraction part(`decimal.scale()!=0`) only, and use an arbitrary-precision `org.apache.kafka.connect.data.Decimal` otherwise.
Also, it updates the method to parse numbers, that can be represented as `float` to `FLOAT64`.
Added unit tests, that cover parsing `BigInteger`, `Byte`, `Short`, `Integer`, `Long`, `Float`, `Double` types.
Reviewers: Konstantine Karantasis <k.karantasis@gmail.com>
To avoid confusion since it is only used by `TestRaftServer`, this PR moves `RaftRequestHandler` to the `tools` package and renames it to `TestRaftRequestHandler`.
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Add following checks to `KafkaConsumer.groupMetadata`:
1. null check of coordinator (replace NPE by `InvalidGroupIdException` which is same to other methods)
2. concurrent check (`groupMetadata` is not thread-safe so concurrent check is necessary)
Reviewers: Jason Gustafson <jason@confluent.io>
System.currentTimeMillis() is not monotonic, so using that to calculate time to sleep can result in negative values. That will throw IllegalArgumentException.
This change checks for that and sleeps for a second (to avoid tight loop) if the value returned is negative.
Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
Migrate different components of the old ProcessorContext interface
into separate interfaces that are more appropriate for their usages.
See KIP-478 for the details.
Reviewers: Guozhang Wang <guozhang@apache.org>, Paul Whalen <pgwhalen@gmail.com>
Ensure that the MM2 checkpoint mirror task replicates consumer offsets even when they are
zero to avoid issues with consumers after failovers.
Author: Andre Araujo <asdaraujo@gmail.com>
Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ryanne Dolan <ryannedolan@gmail.com>, Edoardo Comar <ecomar@uk.ibm.com>, heritamas
Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See https://github.com/FasterXML/jackson-databind/issues/2211 for details of this change.
This change makes recovers the older `JsonConverter` behavior of returning null on empty input.
Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.
Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
Remove the requirement for unique port numbers for the advertised.listener parameters.
This restriction makes for the listeners parameter but there's not reason to apply the
same logic for advertised.listeners.
Being able to do this opens possibilities for some practical applications when using
Kerberos authentication. For example, when configuring Kafka using Kerberos authentication
and a Load Balancer we need to have two SASL_SSL listeners: (A) one running with the
kafka/hostname principal and (B) another using kafka/lb_name, which is necessary for
proper authentication when using the LB FQDN. After bootstrap, though, the client receives
the brokers' addresses with the actual host FQDNs advertised by the brokers. To connect
to the brokerd using the hostnames the client must connect to the listener A to be able to
authenticate successfully with Kerberos.
Author: Andre Araujo <asdaraujo@gmail.com>
Reviewers: Mickael Maison <mickael.maison@gmail.com>, Viktor Somogyi-Vass <viktorsomogyi@gmail.com>, Tom Bentley <tbentley@redhat.com>
Jason Gustafson
huxi
Chris Egerton
John Roesler
Gardner Vickers
Colin Patrick McCabe
Ron Dagostino
Xavier Léauté
voffcheg109
Dima Reznik
Kowshik Prakasam
Chia-Ping Tsai
Jorge Esteban Quilcate Otoya
bill
Lee Dongjin
Michael Bingham
Matthias J. Sax
Sharath Bhat
Nikolay
Gokul Srinivas
Ismael Juma
Rajini Sivaram
Guozhang Wang
Badai Aqrandista
Konstantine Karantasis
Alex Diachenko
Vikas Singh
Ivan Yurchenko
Andre Araujo
Shaik Zakir Hussain
Edoardo Comar