- Only use https links
- Fix broken HTML tags
- Replace usage of <tt> which is deprecated with <code>
- Replace hardcoded version numbers
Reviewers: Chris Egerton <fearthecellos@gmail.com>, Greg Harris <gharris1727@gmail.com>
Part of KIP-714.
Implements ClientTelemetryReporter which manages the lifecycle for client metrics collection. The reporter also defines TelemetrySender which will be used by Network clients to send API calls to broker.
Reviewers: Andrew Schofield <aschofield@confluent.io>, Philip Nee <pnee@confluent.io>, Matthias J. Sax <matthias@confluent.io>
This patch fixes a race condition in the shutdown logic of the `ConsumerNetworkThread`. The `running` variable could be set to `true` after `closeInternal` was called.
Reviewers: Andrew Schofield <aschofield@confluent.io>, Lucas Brutschy <lbrutschy@confluent.io>
This test is flaky because maybeExpediteRefresh schedules a refresh in a background thread. Instead pass through a mock executor service so that the refresh is executed directly.
---------
Co-authored-by: ashwinpankaj <appankaj@amazon.com>
Reviewers: Ashwin Pankaj <apankaj@confluent.io>, Kirk True <ktrue@confluent.io>, Justine Olshan <jolshan@confluent.io>
This pull request is an attempt to get what has started in #12524 to completion as part of the Streams project migration to Mockito.
Reviewers: Divij Vaidya <diviv@amazon.com>, Bruno Cadonna <cadonna@apache.org>
This is part of the investigation into recent build instability. It simply turns off the consumer integration tests that use the new AsyncKafkaConsumer to see whether the build runs smoothly.
Reviewers: David Jacot <djacot@confluent.io>
Verifies that the group ID passed into the async consumer is valid. That is, if the group ID is not null, it is not empty or it does not consist of only whitespaces.
This change stores the group ID in the group metadata because KAFKA-15281 about the group metadata API will build on that.
Reviewers: Lucas Brutschy <lbrutschy@confluent.io>, Kirk True <ktrue@confluent.io>
Caches the maximum time to wait in the consumer network thread so the application thread is better isolated from the request managers.
Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
The following race can happen in the state updater code path
Task is restoring, owned by state updater
We fall out of the consumer group, lose all partitions
We therefore register a "TaskManager.pendingUpdateAction", to CLOSE_DIRTY
We also register a "StateUpdater.taskAndAction" to remove the task
We get the same task reassigned. Since it's still owned by the state updater, we don't do much
The task completes restoration
The "StateUpdater.taskAndAction" to remove will be ignored, since it's already restored
Inside "handleRestoredTasksFromStateUpdater", we close the task dirty because of the pending update action
We now have the task assigned, but it's closed.
To fix this particular race, we cancel the "close" pending update action. Furthermore, since we may have made progress in other threads during the missed rebalance, we need to add the task back to the state updater, to at least check if we are still at the end of the changelog. Finally, it seems we do not need to close dirty here, it's enough to close clean when we lose the task, related to KAFKA-10532.
This should fix the flaky EOSIntegrationTest.
Reviewers: Bruno Cadonna <cadonna@apache.org>
Test startup does not assure that all brokers are registered. In flaky failures,
the `DescribeCluster` API does not return a complete list of brokers. To fix
the issue, we add a call to `ensureConsistentKRaftMetadata()` to ensure that all
brokers are registered and have caught up to current metadata.
Reviewers: David Jacot <djacot@confluent.io>
https://issues.apache.org/jira/browse/KAFKA-14505 is not done yet so we need to disable the system test. Added a comment in the jira to re-enable once it's implemented.
Reviewers: Justine Olshan <jolshan@confluent.io>
In the new consumer, Consumer.poll(Duration timeout) blocks for the entire duration. If the consumer is joining a group and has not yet received its assignments, the poll begins before an assignment has yet been received. Because the poll is blocked, it does not notice when partitions are assigned, and it subsequently does not return any records. The old consumer only blocks for the duration of the heartbeat interval and loops for until the poll timeout has passed, and is thus able to check for assignments received.
When this problem has been fixed, there remains another which prevents the group becoming stable. Because the consumer repeatedly sends the list of topic-partitions that it has been assigned to the group coordinator, the coordinator responds with the list of topic-partitions, which causes the consumer to remain reconciling indefinitely. By making the building of ConsumerGroupHeartbeatRequest stateful, the loop is ended and the group becomes stable as expected.
Reviewers: Lucas Brutschy <lbrutschy@confluent.io>, Kirk True <ktrue@confluent.io>, Lianet Magrans <lianetmr@gmail.com>
This PR fixes some details of the interface to KafkaConsumer.committed which were different between the existing consumer and the new consumer.
Adds a unit test that validates the behaviour is the same for both consumer implementations.
Reviewers: Kirk True <ktrue@confluent.io>, Bruno Cadonna <cadonna@apache.org>
`FetchFromFollowerIntegrationTest.testRackAwareRangeAssignor` is extremely flaky and we have never been able to fix it. This patch disables it until we find a solution to make it reliable with https://issues.apache.org/jira/browse/KAFKA-15020.
Reviewers: Stanislav Kozlovski <stanislav@confluent.io>
When creating partition registrations directories must always be defined.
If creating a partition from a PartitionRecord or PartitionChangeRecord from an older version that
does not support directory assignments, then DirectoryId.MIGRATING is assumed.
If creating a new partition, or triggering a change in assignment, DirectoryId.UNASSIGNED should be
specified, unless the target broker has a single online directory registered, in which case the
replica should be assigned directly to that single directory.
Reviewers: Colin P. McCabe <cmccabe@apache.org>
Implements KIP-992.
Adds TimestampedKeyQuery and TimestampedRangeQuery (IQv2) for ts-ks-store, plus changes semantics of existing KeyQuery and RangeQuery if issues against a ts-kv-store, now unwrapping value-and-timestamp and only returning the plain value.
Reviewers: Matthias J. Sax <matthias@confluent.io>
In KIP-595, we expect to piggy-back on the `quorum.fetch.timeout.ms` config, and if the leader did not receive Fetch requests from a majority of the quorum for that amount of time, it would begin a new election, to resolve the network partition in the quorum. But we missed this implementation in current KRaft. Fixed it in this PR.
The commit include:
1. Added a timer with timeout configuration in `LeaderState`, and check if expired each time when leader is polled. If expired, resigning the leadership and start a new election.
2. Added `fetchedVoters` in `LeaderState`, and update the value each time received a FETCH or FETCH_SNAPSHOT request, and clear it and resets the timer if the majority - 1 of the remote voters sent such requests.
Reviewers: José Armando García Sancio <jsancio@apache.org>
Assign MetadataVersion.IBP_3_7_IV2 to JBOD.
Move KIP-966 support to MetadataVersion.IBP_3_7_IV3.
Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a
new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable
and unstable metadata versions for the first time.
Reviewers: Igor Soarez <soarez@apple.com>, Ron Dagostino <rndgstn@gmail.com>, Calvin Liu <caliu@confluent.io>, Proven Provenzano <pprovenzano@confluent.io>
There is a race in the assertion on `capturedImages`. Since the future is signaled first, it is still possible to see an empty list. By adding to the collection first, we can ensure the assertion will succeed.
Reviewers: Reviewers: David Jacot <djacot@confluent.io>
This interface provides a common supertype for `StreamThread` and
`DefaultTaskExecutor.TaskExecutorThread`, which will be used by KIP-892
to differentiate between "processing" threads and interactive query
threads.
This is needed because `DefaultTaskExecutor.TaskExecutorThread` is
`private`, so cannot be seen directly from `RocksDBStore`.
Reviewer: Bruno Cadonna <cadonna@apache.org>
With `AbstractResponse.maybeSetThrottleTimeMs`, we don't need to use a callback to build the response with the respective throttle.
Reviewers: David Jacot <djacot@confluent.io>
DirectoryId.MIGRATING should be all zeros. All zeros is the default Uuid value in KPRC, and
MIGRATING is the default directory ID value.
Reviewers: Ron Dagostino <rdagostino@confluent.io>
There is no callback associated with autocommit, so I do not think we need this event. This closes KAFKA-15865.
Reviewers: Bruno Cadonna <cadonna@apache.org>
While any blocking operation under holding the UnifiedLog.lock could lead to serious performance (even availability) issues, currently there are several paths that calls fsync(2) inside the lock
In the meantime the lock is held, all subsequent produces against the partition may block
This easily causes all request-handlers to be busy on bad disk performance
Even worse, when a disk experiences tens of seconds of glitch (it's not rare in spinning drives), it makes the broker to unable to process any requests with unfenced from the cluster (i.e. "zombie" like status)
This PR gets rid of 4 cases of essentially-unnecessary fsync(2) calls performed under the lock:
(1) ProducerStateManager.takeSnapshot at UnifiedLog.roll
I moved fsync(2) call to the scheduler thread as part of existing "flush-log" job (before incrementing recovery point)
Since it's still ensured that the snapshot is flushed before incrementing recovery point, this change shouldn't cause any problem
(2) ProducerStateManager.removeAndMarkSnapshotForDeletion as part of log segment deletion
This method calls Utils.atomicMoveWithFallback with needFlushParentDir = true internally, which calls fsync.
I changed it to call Utils.atomicMoveWithFallback with needFlushParentDir = false (which is consistent behavior with index files deletion. index files deletion also doesn't flush parent dir)
This change shouldn't cause problems neither.
(3) LeaderEpochFileCache.truncateFromStart when incrementing log-start-offset
This path is called from deleteRecords on request-handler threads.
Here, we don't need fsync(2) either actually.
On unclean shutdown, few leader epochs might be remained in the file but it will be handled by LogLoader on start-up so not a problem
(4) LeaderEpochFileCache.truncateFromEnd as part of log truncation
Likewise, we don't need fsync(2) here, since any epochs which are untruncated on unclean shutdown will be handled on log loading procedure
Reviewers: Luke Chen <showuon@gmail.com>, Divij Vaidya <diviv@amazon.com>, Justine Olshan <jolshan@confluent.io>, Jun Rao <junrao@gmail.com>
The PR provide implementation for client metrics manager along with other classes. Manager is responsible to support 3 operations:
UpdateSubscription - From kafka-configs.sh and reload from metadata cache.
Process Get Telemetry Request - From KafkaApis.scala
Process Push Telemetry Request - From KafkaApis.scala
Manager maintains an in-memory cache to keep track of client instances against their instance id.
Reviewers: Andrew Schofield <aschofield@confluent.io>, Jun Rao <junrao@gmail.com>
This patch adds support for transactional writes to the CoordinatorRuntime framework. This mainly consists in adding CoordinatorRuntime#scheduleTransactionalWriteOperation and in adding the producerId and producerEpoch to various interfaces. The patch also extends the CoordinatorLoaderImpl and the CoordinatorPartitionWriter accordingly.
Reviewers: Justine Olshan <jolshan@confluent.io>
A few bugs was created from the previous issues. These are:
* During testing or some edge cases, the coordinator request manager might hold on to an inflight request forever. Therefore, when invoking coordinatorRequestManager.poll(), nothing would return. Here we explicitly create a FindCoordinatorRequest regardless of the current request state because we want to actively search for a coordinator
* ensureCoordinatorReady() might be stuck in an infinite loop forever if the client fail to do so. Even the consumer would be able to shutdown eventually, this is undesirable.
* The current asyncConsumerTest mixes background/network thread shutdown with the consumer shutdown. As the goal of the module is unit testing, we should try to test the shutdown procedure separately. Therefore, this PR adds a Mockito.doAnswer call to the applicationEventHandler.close(). Tests that are testing shutdown are calling shutdown() explicitly.
Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
Only add directory.id to meta.properties when migrating to kraft mode, or already in
kraft mode. This prevents incompatibilities with older Kafka releases, which checked
that each directory in a JBOD ensemble had the same meta.properties values.
Reviewers: Colin P. McCabe <cmccabe@apache.org>
The PR outlines classes to collect metrics for client by KafkaMetricsCollector implementation. The MetricsCollector defines mechanism to collect client metrics in sum and gauge metrics format. This requires to define cumulative and delta telemetry metrics while collecting raw metrics.
Singl point metric class helps creating OTLP format Metric object wrapped over Single point metric class itself.
Reviewers: Andrew Schofield <aschofield@confluent.io>, Xavier Léauté <xavier@confluent.io>, Philip Nee <pnee@confluent.io>, Matthias J. Sax <matthias@confluent.io>
The internal topic creation is asynchronous so the test gets flaky. To fix the test flakiness and in this test I want to assert that doesTopicExist should return true when a topic exists, so created a dummy internal topic.
Reviewers: Luke Chen <showuon@gmail.com>, Jun Rao <jun@confluent.io>, Satish Duggana <satishd@apache.org>
- Remove the outdated statement that delegation tokens aren't supported by KRaft.
- Add an invitation to report migration bugs on JIRA.
- Define terminology such as "zk migration phases".
- Mention MV can't be changed during migration.
- Explain how to revert to ZK mode.
Reviewers: Ron Dagostino <rndgstn@gmail.com>, David Arthur <mumrah@gmail.com>
This commit parameterizes the consumer integration tests so they can be run against
the existing "generic" group protocol and the new "consumer" group protocol
introduced in KIP-848.
The KIP-848 client code is under construction so some of the tests do not run on
both variants to start with, but the idea is that the tests can be enabled as the gaps
in functionality are closed.
Reviewers: Lucas Brutschy <lbrutschy@confluent.io>, Kirk True <ktrue@confluent.io>
As described in KAFKA-9470, testBlockOnRequestCompletionFromStateChangeHandler
will block for hours occasionally.
If it passes, it takes 0.5 seconds, so a minute timeout should be safe.
This is not a fix for KAFKA-9470, it's just aiming to make the CI more stable.
Reviewers: David Jacot <djacot@confluent.io>, Matthias J. Sax <matthias@confluent.io>
This patch add the support for static membership to the new consumer group protocol. With a static member can join, re-join, temporarily leave and leave. When a member leaves with the expectation to rejoin, it must rejoin within the session timeout. It is kicks out from the consumer group otherwise.
Reviewers: David Jacot <djacot@confluent.io>
The PR adds support of alter/describe configs for client-metrics as defined in KIP-714
Reviewers: Andrew Schofield <aschofield@confluent.io>, Jun Rao <junrao@gmail.com>
When using older versions of the broker registration RPC, make sure that the new PreviousBrokerEpoch field is set to the default value when building the request object.
Reviewers: David Arthur <mumrah@gmail.com>
Introduce a dummy node connected to every other node and run Bellman-ford from the dummy node once instead of from every node in the graph.
Reviewers: Qichao Chu (@ex172000), Matthias J. Sax <matthias@confluent.io>