A small hotfix to avoid an extra probing rebalance the first time an application is launched.
This should particularly improve the testing experience.
Reviewer: Matthias J. Sax <matthias@confluent.io>, John Roesler <vvcephei@apache.org>
Validate that the assignment is always balanced wrt:
* active assignment balance
* stateful assignment balance
* task-parallel balance
Reviewers: Bruno Cadonna <bruno@confluent.io>, A. Sophie Blee-Goldman <sophie@confluent.io>
Add actual values instead of just passing null in unit tests that check the behavior of the InsertField SMT when trying to insert a field that takes its value from the Kafka record timestamp.
Reviewers: Chris Egerton <chrise@confluent.io>, Konstantine Karantasis <konstantine@confluent.io>
Prior to KAFKA-8106, we allowed the v0 and v1 message formats to contain non-consecutive inner offsets. Inside `LogValidator`, we would detect this case and rewrite the batch. After KAFKA-8106, we changed the logic to raise an error in the case of the v1 message format (v0 was still expected to be rewritten). This caused an incompatibility for older clients which were depending on the looser validation. This patch reverts the old logic of rewriting the batch to fix the invalid inner offsets.
Note that the v2 message format has always had stricter validation. This patch also adds a test case for this.
Reviewers: José Armando García Sancio <jsancio@users.noreply.github.com>, Ismael Juma <ismael@juma.me.uk>
Drops idempotent updates from KTable source operators.
Specifically, drop updates in which the value is unchanged,
and the timestamp is the same or larger.
Implements: KIP-557
Reviewers: Bruno Cadonna <bruno@confluent.io>, John Roesler <vvcephei@apache.org>
* Fix describeConfigs and alterConfigs not to invoke authorizer more
than once
* Add tests to KafkaApisTest to verify the fixes
* Rename `filterAuthorized` to `filterByAuthorized`
* Tweak `filterByAuthorized` to take resources instead of resource
names and improve implementation
* Introduce `partitionMapByAuthorized` and `partitionSeqByAuthorized`
and simplify code by using it
* Replace List with Seq in some AdminManager methods
* Remove stray `println` in `KafkaApisTest`
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
We spotted a case in the soak test where a standby task could be in CREATED state during commit, which causes an illegal state exception. To prevent this from happening, the solution is to always enforce a state check.
Reviewers: Matthias J. Sax <matthias@confluent.io>, John Roesler <vvcephei@apache.org>, Guozhang Wang <wangguoz@gmail.com>
* Use `forEach` instead of `asScala.foreach` for Java Iterables.
* Use `ifPresent` instead of `asScala.foreach` for Java Optionals.
* Use `forEach` instead of `entrySet.forEach` for Java maps.
* Keep `asScala.foreach` for `Properties` as the Scala implementation
has a better interface (keys and values are of type `String`).
* A few clean-ups: unnecessary `()`, `{}`, `new`, etc.
Reviewers: Manikumar Reddy <manikumar@confluent.io>
Generalize the verification in the upgrade test so that it
does not rely on the task assignor's behavior.
Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, John Roesler <vvcephei@apache.org>
This change turns on exact decimal processing in JSON Converter for deserializing decimals, meaning trailing zeros are maintained. Serialization was already using the decimal scale to output the right value, so this change means a value of `1.2300` can now be serialized to JSON and deserialized back to Connect without any loss of information.
Author: Andy Coates <big-andy-coates@users.noreply.github.com>
Reviewers: Randall Hauch <rhauch@gmail.com>, Almog Gavra <almog@confluent.io>
The rest.advertised.listener config is currently broken as setting it to http when listeners are configured for both https and http will cause the framework to choose whichever of the two listeners is listed first. The changes here attempt to fix this by checking not only that ServerConnector::getName begins with the specified protocol, but also that that protocol is immediately followed by an underscore, which the framework uses as a delimiter between the protocol and the remainder of the connector name.
An existing unit test for the RestServer::advertisedUrl method has been expanded to include a case that fails with the framework in its current state and passes with the changes in this commit.
* KAFKA-9768: Fix handling of rest.advertised.listener config
* KAFKA-9768: Add comments on server connector names
* KAFKA-9768: Update RestServerTest comment
Co-authored-by: Randall Hauch <rhauch@gmail.com>
Reviewers: Randall Hauch <rhauch@gmail.com>, Konstantine Karantasis <konstantine@confluent.io>, Andrew Choi <andchoi@linkedin.com>
The CircularIterator class uses a wrapping index-based approach to iterate over a list. This can be a performance problem O(n^2) for a LinkedList. Also, the index counter itself is never reset, a modulo is applied to it for every list access. At some point, it may be possible that the index counter overflows to a negative value and therefore may cause a negative index read and an ArrayIndexOutOfBoundsException.
This fix changes the implementation to avoid these two scenarios. Uses the Collection Iterator classes to avoid using an index counter and it avoids having to seek to the correct index every time, this avoiding the LinkedList performance issue.
I have added unit tests to validate the new implementation.
* KAFKA-9419: Integer Overflow Possible with CircularIterator
* Added JavaDoc. Support null values in the underlying collection
* Always return true for hasNext(). Add more JavaDoc
* Use an advance method to load next value and always return true in hasNext()
* Simplify test suite
* Use assertThrows in tests and remove redundant 'this' identifier
Co-authored-by: David Mollitor <dmollitor@apache.org>
Co-authored-by: Konstantine Karantasis <konstantine@confluent.io>
Reviewers: Ron Dagostino <rdagostino@confluent.io>, Konstantine Karantasis <konstantine@confluent.io>
Currently we add "Data" to all generated classnames in order to avoid naming collisions with existing Request/Response objects. Generated classes for other persistent schema definitions (such as those used in `GroupCoordinator` and `TransactionCoordinator`) will not necessarily have the same problem, so it would be nice if the generated types could use the name defined in the schema directly.
Reviewers: Boyang Chen <boyang@confluent.io>, Colin P. McCabe <cmccabe@apache.org>
This patch ensures that both clients and the bounce schedule get shutdown properly in this test. Additionally, it fixes the surprising behavior of using the passed delivery timeout to override the request timeout in `createTransactionalProducer`.
Reviewers: Ismael Juma <ismael@juma.me.uk>
Previously we had fallback logic when parsing ACLs to handle older entries which may contain non-escaped characters. This code became dead after 1.1 since it was no longer used in the parsing of ACLs. This patch removes the fallback logic.
Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
The version of Zinc included with Gradle 6.4 includes a fix for the blocker
that was preventing us from passing `-release 8` to scalac.
Release notes for Gradle 6.4:
https://docs.gradle.org/6.4/release-notes.html
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
This patch fixes a regression in the `StopReplica` response handling. We should only send the event on receiving the `StopReplica` response if we had requested deletion in the request.
Reviewers: Lucas Bradstreet <lucas@confluent.io>, Jason Gustafson <jason@confluent.io>
* `MockAdminClient` should behave the same way as `Admin` for `createTopics()`
* Changed from throwing an `IllegalArgumentException` to `InvalidReplicationFactorException` when `brokers.size() < replicationFactor`
Author: jeff kim <jeff.kim@confluent.io>
Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
Closes#8617 from jeffkbkim/MockAdminClient-InvalidReplicationFactorException
In the case described in the JIRA, there was a 50%+ increase in the total fetch request rate in
2.4.0 due to this change.
I included a few additional clean-ups:
* Simplify `findPreferredReadReplica` and avoid unnecessary collection copies.
* Use `LongSupplier` instead of `Supplier<Long>` in `SubscriptionState` to avoid unnecessary boxing.
Added a unit test to ReplicaManagerTest and cleaned up the test class a bit including
consistent usage of Time in MockTimer and other components.
Reviewers: Gwen Shapira <gwen@confluent.io>, David Arthur <mumrah@gmail.com>, Jason Gustafson <jason@confluent.io>
Avoid calling into ConfigCommand and TopicCommand from tests that are not related
to these commands. It's better to just invoke the admin APIs.
Change a few cases where we were testing the deprecated --zookeeper flag to testing
the --bootstrap-server flag instead. Unless we're explicitly testing the deprecated code
path, we should be using the non-deprecated flags.
Move testCreateWithUnspecifiedReplicationFactorAndPartitionsWithZkClient from
TopicCommandWithAdminClientTest.scala into TopicCommandWithZKClientTest.scala,
since it makes more sense in the latter.
Reviewers: Colin P. McCabe <cmccabe@apache.org>
This patch ensures that `SslEngineFactory` is closed. The default implementation (**DefaultSslEngineFactory**) does not have any releasable object so we didn't notice this issue. However, it would be better to fix this issue for custom engine factories.
Reviewers: Jason Gustafson <jason@confluent.io>
Comparing all other test cases, the shouldAllowConcurrentAccesses starts an async producer sending records throughout the test other than just synchronously sent and acked a few records before we start the streams application. Right after the streams app is started, we check that at least one record is sent to the output topic (i.e. completed processing). However since only this test starts the producer async and did not wait for it to complete, it is possible that the async producer gets too longer to produce some records and causing it to fail.
To follow what other tests did, I let this test to first send one round of records synchronously before starting the async producing.
Also encountered some new scala warnings that I fixed along with this PR.
Reviewers: Matthias J. Sax <matthias@confluent.io>
KafkaAdminClientTest.testAlterClientQuotas() is uncalled. It is clearly intended to be a test method, but lacks `Test`.
Author: Tom Bentley <tbentley@redhat.com>
Reviewers: Gwen Shapira, Brian Byrne
Closes#8456 from tombentley/MINOR-annotate-test-method
Adjust `checkLogAppendTimeNonCompressed` to assert
`shallowOffsetOfMaxTimestamp` correctly for message format 2.
Reviewers: Ismael Juma <ismael@juma.me.uk>
1. Added a recordInternal function to let all other public functions trigger, so that shouldRecord would only be checked once.
2. In Streams, pass along the current wall-clock time inside InternalProcessorContext when process / punctuate which can be passed in to the record function to reduce the calling frequency of SystemTime.milliseconds().
Reviewers: John Roesler <vvcephei@apache.org>