This patch adds clusterId validation in the `Fetch` API as documented in KIP-595. A new error code `INCONSISTENT_CLUSTER_ID` is returned if the request clusterId does not match the value on the server. If no clusterId is provided, the request is treated as valid.
Reviewers: Jason Gustafson <jason@confluent.io>
Changes how invalid IDs are handled in LeaderAndIsr requests. The ID check now occurs before leader epoch. If the ID exists and is invalid, the partition is ignored and a new `INCONSISTENT_TOPIC_ID` error is returned in the response.
Reviewers: Jason Gustafson <jason@confluent.io>
A Raft-based broker is unable to restart if the broker defers partition
metadata changes for a __consumer_offsets topic-partition. The issue is
that GroupMetadataManager is asked to removeGroupsForPartition() upon
the broker becoming a follower, but in order for that code to function
it requires that the manager's scheduler be started. There are multiple
possible solutions here since removeGroupsForPartition() is a no-op at
this point in the broker startup cycle (nothing has been loaded, so
there is nothing to unload). We could just not invoke the callback. But
it seems more reasonable to not special-case this and instead start
ReplicaManager and the coordinators just before applying the deferred
partitions states.
We also mark deferred partitions for which we are a follower as being
online a bit earlier to avoid NotLeaderOrFollowerException that was
being thrown upon restart. Fixing this issue exposed the above issue
regarding the scheduler not being started.
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ismael Juma <ismael@juma.me.uk>
The `KafkaMetadataLog` implementation of `ReplicatedLog` validates that batches appended using `appendAsLeader` and `appendAsFollower` have an offset that matches the LEO. This is enforced by `KafkaRaftClient` and `BatchAccumulator`. When creating control batches for the `LeaderChangeMessage` the default base offset of `0` was being used instead of using the LEO. This is fixed by:
1. Changing the implementation for `MockLog` to validate against this and throw an `RuntimeException` if this invariant is violated.
2. Always create a batch for `LeaderChangeMessage` with an offset equal to the LEO.
Reviewers: Jason Gustafson <jason@confluent.io>
Previously all APIs were accessible on every listener exposed by the broker, but
with KIP-500, that is no longer true. We now have more complex requirements for
API accessibility.
For example, the KIP-500 controller exposes some APIs which are not exposed by
brokers, such as BrokerHeartbeatRequest, and does not expose most client APIs,
such as JoinGroupRequest, etc. Similarly, the KIP-500 broker does not implement
some APIs that the ZK-based broker does, such as LeaderAndIsrRequest and
UpdateFeaturesRequest.
All of this means that we need more sophistication in how we expose APIs and
keep them consistent with the ApiVersions API. Up until now, we have been
working around this using the controllerOnly flag inside ApiKeys, but this is
not rich enough to support all of the cases listed above. This PR introduces a
new "listeners" field to the request schema definitions. This field is an array
of strings which indicate the listener types in which the API should be exposed.
We currently support "zkBroker", "broker", and "controller". ("broker"
indicates the KIP-500 broker, whereas zkBroker indicates the old broker).
This PR also creates ApiVersionManager to encapsulate the creation of the
ApiVersionsResponse based on the listener type. Additionally, it modifies
SocketServer to check the listener type of received requests before forwarding
them to the request handler.
Finally, this PR also fixes a bug in the handling of the ApiVersionsResponse
prior to authentication. Previously a static response was sent, which means that
changes to features would not get reflected. This also meant that the logic to
ensure that only the intersection of version ranges supported by the controller
would get exposed did not work. I think this is important because some clients
rely on the initial pre-authenticated ApiVersions response rather than doing a
second round after authentication as the Java client does.
One final cleanup note: I have removed the expectation that envelope requests
are only allowed on "privileged" listeners. This made sense initially because
we expected to use forwarding before the KIP-500 controller was available. That
is not the case anymore and we expect the Envelope API to only be exposed on the
controller listener. I have nevertheless preserved the existing workarounds to
allow verification of the forwarding behavior in integration testing.
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ismael Juma <ismael@juma.me.uk>
Introduced by #10113. The error was:
```
14:01:06 > Task :core:compileTestScala
14:01:06 [Error] /home/jenkins/agent/workspace/LoopTest-Kafka/kafka/core/src/test/scala/unit/kafka/server/ControllerApisTest.scala:70: the result type of an implicit conversion must be more specific than Object
```
Reviewers: Ismael Juma <ismael@juma.me.uk
This PR adds the KIP-500 BrokerServer and ControllerServer classes and
makes some related changes to get them working. Note that the ControllerServer
does not instantiate a QuorumController object yet, since that will be added in
PR #10070.
* Add BrokerServer and ControllerServer
* Change ApiVersions#computeMaxUsableProduceMagic so that it can handle
endpoints which do not support PRODUCE (such as KIP-500 controller nodes)
* KafkaAdminClientTest: fix some lingering references to decommissionBroker
that should be references to unregisterBroker.
* Make some changes to allow SocketServer to be used by ControllerServer as
we as by the broker.
* We now return a random active Broker ID as the Controller ID in
MetadataResponse for the Raft-based case as per KIP-590.
* Add the RaftControllerNodeProvider
* Add EnvelopeUtils
* Add MetaLogRaftShim
* In ducktape, in config_property.py: use a KIP-500 compatible cluster ID.
Reviewers: Colin P. McCabe <cmccabe@apache.org>, David Arthur <mumrah@gmail.com>
1. add 'mapKey=true' to DescribeLogDirsRequest
2. rename PartitionIndex to Partitions for DescribeLogDirsRequest
3. add 'mapKey=true' to ElectLeadersRequest
4. rename PartitionId to Partitions for ElectLeadersRequest
5. add 'mapKey=true' to ConsumerProtocolAssignment
Reviewers: David Jacot <djacot@confluent.io>, Ismael Juma <ismael@juma.me.uk>
The script `test-raft-server-start.sh` requires the config to be specified with `--config`. I've included this in the README and added an error message for this specific case.
Reviewers: Jason Gustafson <jason@confluent.io>
We don't really need it and it causes problems in older Android versions
and GraalVM native image usage (there are workarounds for the latter).
Move the logic to separate classes that are only invoked when the
relevant compression library is actually used. Place such classes
in their own package and enforce via checkstyle that only these
classes refer to compression library packages.
To avoid cyclic dependencies, moved `BufferSupplier` to the `utils`
package.
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Add the new KIP-631 configs to KafkaConfigTest to fix the test failure.
Rename InitialBrokerRegistrationTimeoutMs to
InitialBrokerRegistrationTimeoutMsProp for consistency with the other
properties.
Add ControllerListenerNamesProp as specified in KIP-631.
Give nodeId and brokerId the same value in KafkaConfig.
Reviewers: David Arthur <mumrah@gmail.com
This adds BrokerMetadataListener which is responsible for processing metadata records received by the broker when running in Raft mode.
This also moves some classes that were added to the wrong folder in trunk
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ron Dagostino <rdagostino@confluent.io>
Add the KIP-500 broker lifecycle manager. It owns the broker state. Its inputs are
messages passed in from other parts of the broker and from the controller: requests to start
up, or shut down, for example. Its output are the broker state and various futures that can
be used to wait for broker state transitions to occur.
The lifecycle manager handles registering the broker with the controller, as described in
KIP-631. After registration is complete, it handles sending periodic broker heartbeats and
processing the responses.
Reviewers: David Arthur <mumrah@gmail.com>, Ismael Juma <ismael@juma.me.uk>, Ron Dagostino <rdagostino@confluent.io>
This patch implements KIP-635 which mainly adds support for querying offsets of multiple topics/partitions.
Reviewers: David Jacot <djacot@confluent.io>
Currently the partition.metadata file is created when the log is created. However, clusters with older inter-broker protocols will never use this file. This PR moves the creation of the file to when we write to the file.
This PR also deletes the partition.metadata file on startup if the IBP version is lower than 2.8.
Reviewers: Jun Rao <junrao@gmail.com>
This adds the logic to apply partition metadata when consuming from the Raft-based
metadata log.
RaftReplicaManager extends ReplicaManager for now to minimize changes to existing
code for the 2.8 release. We will likely adjust this hierarchy at a later time (e.g. introducing
a trait and adding a helper to refactor common code). For now, we expose the necessary
fields and methods in ReplicaManager by changing their scope from private to protected,
and we refactor out a couple of pieces of logic that are shared between the two
implementation (stopping replicas and adding log dir fetchers).
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ismael Juma <ismael@juma.me.uk>
This PR brings in the new broker metadata processor for handling QuotaRecord-s coming from the metadata log. Also included is a new cache class to allow for fast lookups of quotas on the broker for handling DescribeClientQuotaRequest.
Reviewers: Colin P. McCabe <cmccabe@apache.org>
Rename DecommissionBrokers to UnregisterBrokers. Fix an incorrect JavaDoc comment
for the Admin API. Make sure that UNREGISTER_BROKER is marked as forwardable and
not as a controller-only API (since it can received by brokers).
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, Ismael Juma <ismael@juma.me.uk>
We don't need or use the additional functionality provided by
AtomicReference.
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, David Arthur <mumrah@gmail.com>
Add StorageTool as specified in KIP-631. It can format and describe storage directories. Fix a bug in `ZkMetaProperties#toString`.
Reviewers: David Arthur <mumrah@gmail.com>
Add ClusterTool as specified in KIP-631. It can report the current cluster ID, and also send the new RPC for removing broker registrations.
Reviewers: David Arthur <mumrah@gmail.com>
Currently log recovery begins as soon as we instantiate `LogManager`, but when using a
Raft-based metadata quorum we won't have configs until after we catch up on the metadata
log. We therefore defer log recovery until we actually invoke `startup()` on the `LogManager`
instance. This timing difference has no effect when using ZooKeeper because we
immediately invoke `startup()` on the instantiated instance, but it gives us the necessary
flexibility for accurate log recovery with updated configs when using a Raft-based metadata
quorum.
The `LogCleaner` is currently instantiated during construction just after log recovery
completes, and then it is started in `startup()`. As an extra precaution, since we are
no longer performing recovery during construction, we both instantiate and start the
log cleaner in `startup()` after log recovery completes.
We also convert `LogManager` to use a `ConfigRepository` to load topic configs
(which can override the default log configs) instead of having a hard-coded
dependency on ZooKeeper. We retrieve the topic configs when we invoke `startup()`
-- which again is effectively no different from a timing perspective than what we do
today for the ZooKeeper case.
One subtlety is that currently we create the log configs for every topic at this point
-- if a topic has no config overrides then we associate a copy of the default
configuration with the topic inside a map, and we retrieve the log configs for that
topic's partitions from from that map during recovery. This PR makes a change to
this series of events as follows. We do not associate a copy of the the default
configuration with a topic in the map if the topic has no configs set when we query
for them. This saves some memory -- we don't unnecessarily copy the default
config many times -- but it also means we have use the default log configs for
that topic later on when recovery for each of its partitions begins.
The difference is that the default configs are dynamically reconfigurable, and they
could potentially change between the time when we invoke `startup()` and when
log recovery begins (log recovery can begin quite some time after `startup()` is
invoked if shutdown was unclean). Prior to this patch such a change would not
be used; with this patch they could be if they happen before recovery begins.
This actually is better -- we are performing log recovery with the most recent
known defaults when a topic had no overrides at all. Also, `Partition.createLog`
has logic to handle missed config updates, so the behavior is eventually the same.
The transition of the broker state from `STARTING` to `RECOVERY` currently
happens within the `LogManager`, and it only occurs if the shutdown was
unclean. We move this transition into the broker as it avoids passing a
reference to the broker state into the `LogManager`. We also now always
transition the broker into the `RECOVERY` state as dictated by [the KIP-631 broker state machine](https://cwiki.apache.org/confluence/display/KAFKA/KIP-631%3A+The+Quorumbased+Kafka+Controller#KIP631:TheQuorumbasedKafkaController-TheBrokerStateMachine).
Finally, a few clean-ups were included. One worth highlighting is that `Partition`
no longer requires a `ConfigRepository`.
Reviewers: David Arthur <david.arthur@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Consolidate auto topic creation logic to either forward a CreateTopicRequest or handling the creation directly as AutoTopicCreationManager, when handling FindCoordinator/Metadata request.
Co-authored-by: Jason Gustafson <jason@confluent.io>
Reviewers: Jason Gustafson <jason@confluent.io>
Refactor the MetadataCache into two implementations that both implement a common trait. This will let us
continue to use the existing implementation when using ZK, but use the new implementation when in kip-500 mode.
Reviewers: Colin McCabe <cmccabe@apache.org>, Justine Olshan <jolshan@confluent.io>, Jason Gustafson <jason@confluent.io>
`KafkaApis` is configured differently when it is used in a broker with a Raft-based controller quorum vs. a ZooKeeper quorum. For example, when using Raft, `ForwardingManager` is required rather than optional, and there is no `AdminManager`, `KafkaController`, or `KafkaZkClient`. This PR introduces `MetadataSupport` to abstract the two possibilities: `ZkSupport` and `RaftSupport`. This provides a fluent way to decide what to do based on the type of support that `KafkaApis` has been configured with. Certain types of requests are not supported when using raft (`AlterIsrRequest`, `UpdateMetadataRequest`, etc.), and `MetadataSupport` gives us an intuitive way to identify the constraints and requirements associated with the different configurations and react accordingly.
Existing tests are sufficient to detect bugs and regressions.
Reviewers: José Armando García Sancio <jsancio@gmail.com>, Jason Gustafson <jason@confluent.io>
The group coordinator and the transaction state manager query ZooKeeper
to retrieve the partition count for the topics they manager. Since ZooKeeper
won't be available when the broker is using a Raft-based metadata quorum,
this PR changes the startup function to provide an accessor function instead.
This will allow the ZK-based broker to continue using ZK, while the kip-500
broker will query the metadata provided by the metadata log.
Reviewers: Colin P. McCabe <cmccabe@apache.org>, David Arthur <david.arthur@confluent.io>
Co-authored-by: Ismael Juma <ismael@juma.me.uk>
`Partition` objects are able to retrieve topic configs when creating their log, and currently they do so with an implementation of `trait TopicConfigFetcher` that uses ZooKeeper. ZooKeeper is not available when using a Raft-based metadata log, so we need to abstract the retrieval of configs so it can work either with or without ZooKeeper. This PR introduces `trait ConfigRepository` with `ZkConfigRepository` and `CachedConfigRepository` implementations. `Partition` objects now use a provided `ConfigRepository` to retrieve topic configs, and we eliminate `TopicConfigFetcher` as it is no longer needed.
`ReplicaManager` now contains an instance of `ConfigRepository` so it can provide it when creating `Partition` instances.
`KafkaApis` needs to be able to handle describe-config requests; it currently delegates that to `ZkAdminManager`, which of course queries ZooKeeper. To make this work with or without ZooKeeper we move the logic from `ZkAdminManager` into a new `ConfigHelper` class that goes through a `ConfigRepository` instance. We provide `KafkaApis` with such an instance, and it creates an instance of the helper so it can use that instead of going through `ZkAdminManager`.
Existing tests are sufficient to identify bugs and regressions in `Partition`, `ReplicaManager`, `KafkaApis`, and `ConfigHelper`. The `ConfigRepository` implementations have their own unit tests.
Reviewers: Jason Gustafson <jason@confluent.io>
Dynamic broker reconfiguration needs to occur for both ZooKeeper-based
brokers and brokers that use a Raft-based metadata quorum. DynamicBrokerConfig
currently operates on KafkaServer, but it needs to operate on BrokerServer
(the broker implementation that will use the Raft metadata log) as well.
This PR introduces a KafkaBroker trait to allow dynamic reconfiguration to
work with either implementation.
Reviewers: José Armando García Sancio <jsancio@gmail.com>, Colin Patrick McCabe <cmccabe@confluent.io>, Ismael Juma <ismael@juma.me.uk>
This patch adds a `RecordSerde` implementation for the metadata record format expected by KIP-631.
Reviewers: Colin McCabe <cmccabe@apache.org>, Ismael Juma <mlists@juma.me.uk>
Three new classes are added to support the upcoming changes to MetadataCache
required for handling Raft metadata records.
Reviewers: Jason Gustafson <jason@confluent.io>
Co-authored-by: David Arthur <mumrah@gmail.com>
Replace BrokerStates.scala with BrokerState.java, to make it easier to use from Java code if needed. This also makes it easier to go from a numeric type to an enum.
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Tests involving `BrokerToControllerChannelManager` are simplified by being able to leverage `MockClient`. This patch introduces a `MockBrokerToControllerChannelManager` implementation which makes that possible.
The patch updates `ForwardingManagerTest` to use `MockBrokerToControllerChannelManager`. We also add a couple additional timeout cases, which exposed a minor bug. Previously we were using the wrong `TimeoutException`, which meant that expected timeout errors were in fact translated to `UNKNOWN_SERVER_ERROR`.
Reviewers: David Arthur <david.arthur@confluent.io>
`ProducerIdManager` is an existing class that talks to ZooKeeper directly. We won't have ZooKeeper
when using a Raft-based metadata quorum, so we need an abstraction for the functionality of
generating producer IDs. This PR introduces `ProducerIdGenerator` for this purpose, and we pass
an implementation when instantiating `TransactionCoordinator` rather than letting
`TransactionCoordinator.apply()` itself always create a ZooKeeper-based instance.
Reviewers: David Arthur <mumrah@gmail.com>, Ismael Juma <ismael@juma.me.uk>
Brokers receive metadata from the Raft metadata quorum very differently than they do from
ZooKeeper today, and this has implications for ReplicaManager. In particular, when a broker
reads the metadata log it may not arrive at the ultimate state for a partition until it reads multiple
messages. In normal operation the multiple messages associated with a state change will all
appear in a single batch, so they can and will be coalesced and applied together. There are
circumstances where messages associated with partition state changes will appear across
multiple batches and we will be forced to coalesce these multiple batches together. The
circumstances when this occurs are as follows:
- When the broker restarts it must "catch up" on the metadata log, and it is likely that the
broker will see multiple partition state changes for a single partition across different
batches while it is catching up. For example, it will see the `TopicRecord` and the
`PartitionRecords` for the topic creation, and then it will see any `IsrChangeRecords`
that may have been recorded since the creation. The broker does not know the state of
the topic partitions until it reads and coalesces all the messages.
- The broker will have to "catch up" on the metadata log if it becomes fenced and then
regains its lease and resumes communication with the metadata quorum.
- A fenced broker may ultimately have to perform a "soft restart" if it was fenced for so
long that the point at which it needs to resume fetching the metadata log has been
subsumed into a metadata snapshot and is no longer independently fetchable. A soft
restart will entail some kind of metadata reset based on the latest available snapshot
plus a catchup phase to fetch after the snapshot end point.
The first case -- during startup -- occurs before clients are able to connect to the broker.
Clients are able to connect to the broker in the second case. It is unclear if clients will be
able to to connect to the broker during a soft restart (the third case).
We need a way to defer the application of topic partition metadata in all of the above cases,
and while we are deferring the application of the metadata the broker will not service clients
for the affected partitions.
As a side note, it is arguable if the broker should be able to service clients while catching up
or not. The decision to not service clients has no impact in the startup case -- clients can't
connect yet at that point anyway. In the third case it is not yet clear what we are going to do,
but being unable to service clients while performing a soft reset seems reasonable. In the
second case it is most likely true that we will catch up quickly; it would be unusual to
reestablish communication with the metadata quorum such that we gain a new lease and
begin to catch up only to lose our lease again.
So we need a way to defer the application of partition metadata and make those partitions
unavailable while deferring state changes. This PR adds a new internal partition state to
ReplicaManager to accomplish this. Currently the available partition states are simple
`Online`, `Offline` (meaning a log dir failure) and `None` (meaning we don't know about it).
We add a new `Deferred` state. We also rename a couple of methods that refer to
"nonOffline" partitions to instead refer to "online" partitions.
**The new `Deferred` state never happens when using ZooKeeper for metadata storage.**
Partitions can only enter the `Deferred` state when using a KIP-500 Raft metadata quorum
and one of the above 3 cases occurs. The testing strategy is therefore to leverage existing
tests to confirm that there is no functionality change in the ZooKeeper case. We will add
the logic for deferring/applying/reacting to deferred partition state in separate PRs since
that code will never be invoked in the ZooKeeper world.
Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk>
This PR moves static property definitions for user client quotas into a
new class called QuotaConfigs in the clients module under the
o.a.k.common.config.internals package. This is needed to support the
client quotas work in the quorum based controller.
Reviewers: Colin McCabe <cmccabe@apache.org>
mTLS is enabled if listener-prefixed ssl.client.auth is configured for SASL_SSL listeners. Broker-wide ssl.client.auth is not applied to SASL_SSL listeners as before, but we now print a warning.
Reviewers: David Jacot <djacot@confluent.io>
This patch adds the schemas and request/response objects for the `BrokerHeartbeat` and `BrokerRegistration` APIs that were added as part of KIP-631. These APIs are only exposed by the KIP-500 controller and not advertised by the broker.
Reviewers: Jason Gustafson <jason@confluent.io>
This patch factors out a trait to allow for other ways to provide the controller `Node` object to `BrokerToControllerChannelManager`. In KIP-500, the controller will be provided from the Raft client and not the metadata cache.
Reviewers: David Arthur <david.arthur@confluent.io>
This patch attempts to fix `CustomQuotaCallbackTest#testCustomQuotaCallback`. The test creates 99 partitions in a topic, and expects that we can get the partition info for all of them after 15 seconds. If we cannot, then we'll get the error:
```
org.scalatest.exceptions.TestFailedException: Partition [group1_largeTopic,69] metadata not propagated after 15000 ms
```
15 secs is not enough to complete the 99 partitions creation on a slow system. So, we fix it by explicitly wait until we've got the expected partition size before retrieving each partition info and we increase the wait time to 60s for all partition metadata to be propagated.
Reviewers: Jason Gustafson <jason@confluent.io>
Since the Raft leader is already doing the work of assigning offsets and the leader epoch, we can skip the same logic in `Log.appendAsLeader`. This lets us avoid an unnecessary round of decompression.
Reviewers: dengziming <dengziming1993@gmail.com>, Jason Gustafson <jason@confluent.io>
This patch contains the new handling of `meta.properties` required by the KIP-500 server as specified in KIP-631. When using the self-managed quorum, the `meta.properties` file is required in each log directory with the new `version` property set to 1. It must include the `cluster.id` property and it must have a `node.id` matching that in the configuration.
The behavior of `meta.properties` for the Zookeeper-based `KafkaServer` does not change. We treat `meta.properties` as optional and as if it were `version=0`. We continue to generate the clusterId and/or the brokerId through Zookeeper as needed.
Reviewers: Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
David Jacot
Justine Olshan
Ron Dagostino
José Armando García Sancio
Jason Gustafson
Chia-Ping Tsai
runom
Ismael Juma
Colin Patrick McCabe
David Arthur
Dániel Urbán
Kamal Chandraprakash
dengziming
Boyang Chen
Alok Nikhil
Rajini Sivaram
Luke Chen
feyman2016