Reduces overheads by avoiding re-calling time.milliseconds().
Author: Eno Thereska <eno.thereska@gmail.com>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#2583 from enothereska/minor-reduce-milliseconds
* Turned 3 inner classes that weren't static but could be into `static` ones.
* Turned one `public` inner class that wasn't used publicly into a `private`.
Trivial but imo worthwhile to explicitly keep visibility and "staticness" correct in syntax (if only to be nice to the GC) :)
Author: Armin Braun <me@obrown.io>
Reviewers: Matthias J. Sax <matthias@confluent.io>, Jason Gustafson <jason@confluent.io>
Closes#2574 from original-brownbear/cleanup-inner-nonstatic
…lass should be static
Author: Colin P. Mccabe <cmccabe@confluent.io>
Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk>
Closes#2558 from cmccabe/KAFKA-4774
Currently the checkpoint file is deleted at state store initialization and it is only ever written again during a clean shutdown. This can result in significant delays during restarts as the entire store needs to be loaded from the changelog.
We can mitigate against this by frequently checkpointing the offsets. The checkpointing happens only during the commit phase, i.e, after we have manually flushed the store and the producer. So we guarantee that the checkpointed offsets are never greater than what has been flushed.
In the event of hard failure we can recover by reading the checkpoints and consuming from the stored offsets.
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Eno Thereska, Matthias J. Sax, Guozhang Wang
Closes#2471 from dguy/kafka-4317
More details:
* Replaced `struct` field in Request/Response with a `toStruct` method. This
makes the performance model (including memory usage) easier to understand.
Note that requests have `toStruct()` while responses have `toStruct(version)`.
* Replaced mutable `version` field in `Request.Builder` with an immutable
field `desiredVersion` and a `version` parameter passed to the `build` method.
* Optimised `handleFetchRequest` to avoid unnecessary creation of `Struct`
instances (from 4 to 2 in the worst case and 2 to 1 in the best case).
* Various clean-ups in request/response classes and their test. In particular,
it is now clear what we are testing. Previously, it looked like we were testing
more than we really were.
With this in place, we could remove `AbstractRequest.Builder` in the future by
doing the following:
* Change `AbstractRequest.toStruct` to accept a version (like responses).
* Change `AbstractRequest.version` to be `desiredVersion` (like `Builder`).
* Change `ClientRequest` to take `AbstractRequest`.
* Move validation from the `build` methods to the request constructors or
static factory methods.
* Anything else required for the code to compile again.
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Apurva Mehta <apurva.1618@gmail.com>, Jason Gustafson <jason@confluent.io>
Closes#2513 from ijuma/separate-struct
Lowered the default RocksDB settings for the block cache and write buffers
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Eno Thereska, Guozhang Wang
Closes#2525 from dguy/kafka-4484
https://issues.apache.org/jira/browse/KAFKA-4720
Peek is a handy method to have to insert diagnostics that do not affect the stream itself, but some external state such as logging or metrics collection.
Author: Steven Schlansker <sschlansker@opentable.com>
Reviewers: Damian Guy, Matthias J. Sax, Eno Thereska, Guozhang Wang
Closes#2493 from stevenschlansker/kafka-4720-peek
Add a note to `KafkaStreams.metadataForKey(String, K, Serializer<K>)` to point out that in the case of a Window Store the Serializer should still be the record key serializer and not a window serializer
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Xavier Léauté, Guozhang Wang
Closes#2532 from dguy/minor-docs
This PR fixes a blocker issue, where the streams client code cannot talk to the controller. It also enables a system test that was previously failing.
This PR is for trunk only. A separate PR with just the fix (but not the tests) will be created for 0.10.2.
Author: Eno Thereska <eno@confluent.io>
Author: Eno Thereska <eno.thereska@gmail.com>
Reviewers: Damian Guy, Ismael Juma, Matthias J. Sax, Guozhang Wang
Closes#2522 from enothereska/KAFKA-4716-metadata
Author: Eno Thereska <eno.thereska@gmail.com>
Author: Eno Thereska <eno@confluent.io>
Author: Ubuntu <ubuntu@ip-172-31-22-146.us-west-2.compute.internal>
Reviewers: Matthias J. Sax, Guozhang Wang
Closes#2478 from enothereska/minor-benchmark-args
Provide test coverage for exception paths in: `schedule()`, `closeTopology()`, and `punctuate()`
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Matthias J. Sax, Guozhang Wang
Closes#2451 from dguy/kafka-4640
Removed readKeyValues() that give UNLIMITED_MESSAGES which will doom to exhaust all wait time, as all its callers actually do provide the expected number of messages.
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Matthias J. Sax <matthias@confluent.io>, Jason Gustafson <jason@confluent.io>
Closes#2507 from guozhangwang/KHotfix-not-use-limited-num-messages
This resolves an issue in driving tests using the ProcessorTopologyTestDriver when `groupBy()` is invoked downstream of a processor that flags repartitioning.
Ticket: https://issues.apache.org/jira/browse/KAFKA-4461
Discussion: http://search-hadoop.com/m/Kafka/uyzND1wbKeY1Q8nH1
dguy guozhangwang
The contribution is my original work and I license the work to the project under the project's open source license.
Author: Adrian McCague <amccague@gmail.com>
Reviewers: Damian Guy, Guozhang Wang
Closes#2499 from amccague/KAFKA-4461_ProcessorTopologyTestDriver_map_groupbykey
Delay the cleanup of state directories that are not locked and not owned by the current thread such that we only remove the directory if its last modified is < now - cleanupDelayMs.
This also helps to avoid a race between threads on the same instance, where during rebalance, one thread releases the lock on the state directory, and before another thread can take the lock, the cleanup runs and removes the data.
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Matthias J. Sax, Guozhang Wang
Closes#2486 from dguy/KAFKA-4724
This is a refactoring follow-up of https://github.com/apache/kafka/pull/2166. Main refactoring changes:
1. Extract `InMemoryKeyValueStore` out of `InMemoryKeyValueStoreSupplier` and remove its duplicates in test package.
2. Add two abstract classes `AbstractKeyValueIterator` and `AbstractKeyValueStore` to collapse common functional logics.
3. Added specialized `BytesXXStore` to accommodate cases where key value types are Bytes / byte[] so that we can save calling the dummy serdes.
4. Make the key type in `ThreadCache` from byte[] to Bytes, as SessionStore / WindowStore's result serialized bytes are in the form of Bytes anyways, so that we can save unnecessary `Bytes.get()` and `Bytes.wrap(bytes)`.
Each of these should arguably be a separate PR and I apologize for the mess, this is because this branch was extracted from a rather large diff that has multiple refactoring mingled together and dguy and myself have already put lots of efforts to break it down to a few separate PRs, and this is the only left-over work. Such PR won't happen in the future.
Ping dguy enothereska mjsax for reviews
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Damian Guy, Matthias J. Sax, Jun Rao
Closes#2333 from guozhangwang/K3452-followup-state-store-refactor
Exception paths in `register()`, `topic()`, `partition()`, `offset()`, and `timestamp()`, were not covered by any existing tests
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Eno Thereska, Matthias J. Sax
Closes#2447 from dguy/KAFKA-4646
Add coverage for exception paths in `initialize()`
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Eno Thereska, Matthias J. Sax, Guozhang Wang
Closes#2452 from dguy/kafka-4649
…tReset
Author: bbejeck <bbejeck@gmail.com>
Reviewers: Damian Guy, Matthias J. Sax, Guozhang Wang
Closes#2464 from bbejeck/KAFKA-4662_improve_topology_builder_test_coverage
Author: Maysam Yabandeh <myabandeh@dropbox.com>
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jun Rao <junrao@gmail.com>
Closes#2474 from ijuma/kafka-4039-deadlock-during-shutdown
The issue of transiently having duplicates is due to the bad design of the left join itself: in order to ignore the partial joined results such as `A:null`, it lets the producer to potentially send twice to source stream one and rely on all the following conditions to be true in order to pass the test:
1. `receiveMessages` happen to have fetched all the produced results and have committed offsets.
2. streams app happen to have completed sending all result data.
3. consumer used in `receiveMessages` will complete getting all messages in a single poll().
If any of the above is not true, the test fails.
Fixed this test to add a filter right after left join to filter out partial joined results. Minor cleanup on integration test utils.
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Damian Guy, Ewen Cheslack-Postava
Closes#2485 from guozhangwang/K3896-duplicate-join-results
the toString method prints the topology, but had no tests making sure it works and/or doesn't cause exceptions
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Matthias J. Sax, Eno Thereska, Guozhang Wang
Closes#2444 from dguy/KAFKA-4645
Add a test to ensure a `StreamsException` is thrown when an exception other than `StreamsException` is caught
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Matthias J. Sax, Guozhang Wang
Closes#2450 from dguy/KAFKA-4647
Most of the exception paths weren't covered. Now they are.
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Eno Thereska, Guozhang Wang
Closes#2442 from dguy/KAFKA-4642
Kafka brokers have a config called "offsets.topic.replication.factor" that specify the replication factor for the "__consumer_offsets" topic. The problem is that this config isn't being enforced. If an attempt to create the internal topic is made when there are fewer brokers than "offsets.topic.replication.factor", the topic ends up getting created anyway with the current number of live brokers. The current behavior is pretty surprising when you have clients or tooling running as the cluster is getting setup. Even if your cluster ends up being huge, you'll find out much later that __consumer_offsets was setup with no replication.
The cluster not meeting the "offsets.topic.replication.factor" requirement on the internal topic is another way of saying the cluster isn't fully setup yet.
The right behavior should be for "offsets.topic.replication.factor" to be enforced. Topic creation of the internal topic should fail with GROUP_COORDINATOR_NOT_AVAILABLE until the "offsets.topic.replication.factor" requirement is met. This closely resembles the behavior of regular topic creation when the requested replication factor exceeds the current size of the cluster, as the request fails with error INVALID_REPLICATION_FACTOR.
Author: Onur Karaman <okaraman@linkedin.com>
Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk>, Ewen Cheslack-Postava <ewen@confluent.io>
Closes#2177 from onurkaraman/KAFKA-3959
Makes task assignment more sticky by preferring to assign tasks to clients that had previously had the task as active task. If there are no clients with the task previously active, then search for a standby. Finally falling back to the least loaded client.
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Matthias J. Sax, Guozhang Wang
Closes#2429 from dguy/kafka-4677
Author: Jeyhun Karimov <je.karimov@gmail.com>
Reviewers: Matthias J. Sax, Eno Thereska, Damian Guy, Guozhang Wang
Closes#2420 from jeyhunkarimov/KAFKA-4613
- also deprecate ZK config for Streams
Author: Matthias J. Sax <matthias@confluent.io>
Reviewers: Ismael Juma, Guozhang Wang
Closes#2459 from mjsax/javaDocImprovements8
In some Places for the loop was used but it can be replaced by the for each.
In One file if else if else was used so I replaced the same with match.
Author: Akash Sethi <akash.sethi@knoldus.in>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#2435 from akashsethi24/trunk
Some exception paths not previously covered. Extracted `ensureCopartitioning` into a static class.
Author: Damian Guy <damian.guy@gmail.com>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#2448 from dguy/KAFKA-4644
Author: Matthias J. Sax <matthias@confluent.io>
Reviewers: Damian Guy, Eno Thereska, Guozhang Wang
Closes#2403 from mjsax/addStreamsClientCompatibilityTest
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Jun Rao <junrao@gmail.com>, Rajini Sivaram <rajinisivaram@googlemail.com>
Closes#2406 from ijuma/kafka-4636-per-listener-security-settings
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Eno Thereska <eno.thereska@gmail.com>, Xavier Léauté <xavier@confluent.io>, Jason Gustafson <jason@confluent.io>
Closes#2454 from guozhangwang/KMinor-trace-logging-add-metrics-twice
Author: Ismael Juma <ismael@juma.me.uk>
Reviewers: Eno Thereska, Damian Guy, Guozhang Wang
Closes#2441 from ijuma/streams-kafka-client-drops-security-configs
`bin-kafka-console-producer.sh` should be `bin/kafka-console-producer.sh`.
Author: Will Marshall <wcm214@gmail.com>
Reviewers: Guozhang Wang <wangguoz@gmail.com>
Closes#2410 from wmarshall484/typo-fix
This is the HOTFIX PR for any issues detected with KIP-104 until code freeze. Note: do not merge until close to code freeze.
The name changes reflect feedback received while writing the documentation.
Author: Eno Thereska <eno.thereska@gmail.com>
Reviewers: Dan Norwood, Guozhang Wang
Closes#2398 from enothereska/hotfix-streams-metrics
Found a few recently added unit tests did not close KStreamTestDriver after the test itself is closed; this can cause RocksDB virtual function called if the contained topology has some persistent store since they will be initialized but not closed in time.
MINOR fix: found that when closing KStreamTestDriver, we need to first flushing all stores before closing any of them; this is triggered from the `KTableKTableLeftJoin.shouldNotThrowIllegalStateExceptionWhenMultiCacheEvictions`.
MINOR fix: in CachingXXXStore, the `name` field is actually used as the cache's namespace, not really the store name or its corresponding topic name. Fixed it by renaming it to `cacheName` and use `this.name()` elsewhere which will call the underlying store's name.
Author: Guozhang Wang <wangguoz@gmail.com>
Reviewers: Ewen Cheslack-Postava, Eno Thereska, Damian Guy
Closes#2432 from guozhangwang/K3502-kstream-builder-test
Eno Thereska
Eno Thereska
Armin Braun
Colin P. Mccabe
Damian Guy
Ismael Juma
Steven Schlansker
Bill Bejeck
Jonathan Monette
Guozhang Wang
Adrian McCague
Maysam Yabandeh
Onur Karaman
Matthias J. Sax
Jeyhun Karimov
Akash Sethi
Will Marshall
Gwen Shapira