[jira] [Created] (KAFKA-1620) Make kafka api protocol implementation public
Anton Karamanov created KAFKA-1620:
--
Summary: Make kafka api protocol implementation public
Key: KAFKA-1620
URL: https://issues.apache.org/jira/browse/KAFKA-1620
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Some of the classes which implement Kafka api protocol, such as
{{RequestOrResponse}} and {{FetchRequest}} are defined as private to {{kafka}}
package. Those classes would be extremely usefull for writing custom clients
(we're using Scala with Akka and implementing one directly on top of Akka TCP),
and don't seem to contain any actuall internal logic of Kafka. Therefore it
seems like a nice idea to make them public.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (KAFKA-1620) Make kafka api protocol implementation public
[
https://issues.apache.org/jira/browse/KAFKA-1620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Karamanov updated KAFKA-1620:
---
Attachment: 0001-KAFKA-1620-Make-kafka-api-protocol-implementation-pu.patch
Here's a small
[patch|^0001-KAFKA-1620-Make-kafka-api-protocol-implementation-pu.patch] to
address the issue.
Make kafka api protocol implementation public
-
Key: KAFKA-1620
URL: https://issues.apache.org/jira/browse/KAFKA-1620
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Attachments:
0001-KAFKA-1620-Make-kafka-api-protocol-implementation-pu.patch
Some of the classes which implement Kafka api protocol, such as
{{RequestOrResponse}} and {{FetchRequest}} are defined as private to
{{kafka}} package. Those classes would be extremely usefull for writing
custom clients (we're using Scala with Akka and implementing one directly on
top of Akka TCP), and don't seem to contain any actuall internal logic of
Kafka. Therefore it seems like a nice idea to make them public.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
Re: [jira] [Updated] (KAFKA-1620) Make kafka api protocol implementation public
I'm curiosity why Kafka don't implementation protocol by protocol buffer or
any other tools . It's good to use by other language
2014-09-01 22:48 GMT+08:00 Anton Karamanov (JIRA) j...@apache.org:
[
https://issues.apache.org/jira/browse/KAFKA-1620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Karamanov updated KAFKA-1620:
---
Reviewer: Jun Rao
Assignee: Anton Karamanov
Status: Patch Available (was: Open)
Make kafka api protocol implementation public
-
Key: KAFKA-1620
URL: https://issues.apache.org/jira/browse/KAFKA-1620
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1620-Make-kafka-api-protocol-implementation-pu.patch
Some of the classes which implement Kafka api protocol, such as
{{RequestOrResponse}} and {{FetchRequest}} are defined as private to
{{kafka}} package. Those classes would be extremely usefull for writing
custom clients (we're using Scala with Akka and implementing one directly
on top of Akka TCP), and don't seem to contain any actuall internal logic
of Kafka. Therefore it seems like a nice idea to make them public.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
--
long is the way and hard that out of Hell leads up to light
[jira] [Updated] (KAFKA-1621) Standardize --messages option in perf scripts
[ https://issues.apache.org/jira/browse/KAFKA-1621?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jay Kreps updated KAFKA-1621: - Labels: newbie (was: ) Standardize --messages option in perf scripts - Key: KAFKA-1621 URL: https://issues.apache.org/jira/browse/KAFKA-1621 Project: Kafka Issue Type: Bug Affects Versions: 0.8.1.1 Reporter: Jay Kreps Labels: newbie This option is specified in PerfConfig and is used by the producer, consumer and simple consumer perf commands. The docstring on the argument does not list it as required but the producer performance test requires it--others don't. We should standardize this so that either all the commands require the option and it is marked as required in the docstring or none of them list it as required. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (KAFKA-1621) Standardize --messages option in perf scripts
[ https://issues.apache.org/jira/browse/KAFKA-1621?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jay Kreps updated KAFKA-1621: - Affects Version/s: 0.8.1.1 Standardize --messages option in perf scripts - Key: KAFKA-1621 URL: https://issues.apache.org/jira/browse/KAFKA-1621 Project: Kafka Issue Type: Bug Affects Versions: 0.8.1.1 Reporter: Jay Kreps Labels: newbie This option is specified in PerfConfig and is used by the producer, consumer and simple consumer perf commands. The docstring on the argument does not list it as required but the producer performance test requires it--others don't. We should standardize this so that either all the commands require the option and it is marked as required in the docstring or none of them list it as required. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Review Request 25236: Patch for KAFKA-1619
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25236/ --- Review request for kafka. Bugs: KAFKA-1619 https://issues.apache.org/jira/browse/KAFKA-1619 Repository: kafka Description --- remove project perf from build script Diffs - build.gradle 6d6f1a4349d71bd1b56b7cc5c450046a75a83d6e settings.gradle 6041784d6f84c66bb1e9df2bc112e1b06c0bb000 Diff: https://reviews.apache.org/r/25236/diff/ Testing --- Thanks, Jun Rao
Re: Review Request 25236: Patch for KAFKA-1619
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25236/ --- (Updated Sept. 1, 2014, 5:40 p.m.) Review request for kafka. Bugs: KAFKA-1619 https://issues.apache.org/jira/browse/KAFKA-1619 Repository: kafka Description (updated) --- fix readme Diffs (updated) - README.md 8cd5cfd1e04dbef3c0878ff477eebea9ac749233 build.gradle 6d6f1a4349d71bd1b56b7cc5c450046a75a83d6e settings.gradle 6041784d6f84c66bb1e9df2bc112e1b06c0bb000 Diff: https://reviews.apache.org/r/25236/diff/ Testing --- Thanks, Jun Rao
Re: Review Request 25236: Patch for KAFKA-1619
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25236/ --- (Updated Sept. 1, 2014, 5:55 p.m.) Review request for kafka. Bugs: KAFKA-1619 https://issues.apache.org/jira/browse/KAFKA-1619 Repository: kafka Description (updated) --- fix readme fix typo in readme Diffs (updated) - README.md 8cd5cfd1e04dbef3c0878ff477eebea9ac749233 build.gradle 6d6f1a4349d71bd1b56b7cc5c450046a75a83d6e settings.gradle 6041784d6f84c66bb1e9df2bc112e1b06c0bb000 Diff: https://reviews.apache.org/r/25236/diff/ Testing --- Thanks, Jun Rao
[jira] [Created] (KAFKA-1622) project shouldn't require signing to build
Joe Stein created KAFKA-1622: Summary: project shouldn't require signing to build Key: KAFKA-1622 URL: https://issues.apache.org/jira/browse/KAFKA-1622 Project: Kafka Issue Type: Bug Reporter: Joe Stein Fix For: 0.8.2 we only need signing for uploadArchives that is it The project trunk failed to build due to some signing/license checks (the diff I used to get things to build is here: https://gist.github.com/dehora/7e3c0bd75bb2b5d87557) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (KAFKA-1282) Disconnect idle socket connection in Selector
[ https://issues.apache.org/jira/browse/KAFKA-1282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14117638#comment-14117638 ] nicu marasoiu edited comment on KAFKA-1282 at 9/1/14 8:18 PM: -- I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. Since the last rebase, I cannot compile test anymore! Building 0% :test_core_2_10_1 :kafka:core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder, it happens when i checkout an old revision - seems to be something with gradle or scala changed in my local environment, although i did not make any explicit update or upgrade; i use Intellij). was (Author: nmarasoi): I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. Since the last rebase, I cannot compile test anymore! Building 0% :test_core_2_10_1 :kafka:core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder). Disconnect idle socket connection in Selector - Key: KAFKA-1282 URL: https://issues.apache.org/jira/browse/KAFKA-1282 Project: Kafka Issue Type: Bug Components: producer Affects Versions: 0.8.2 Reporter: Jun Rao Assignee: Neha Narkhede Labels: newbie++ Fix For: 0.9.0 Attachments: KAFKA-1282_Disconnect_idle_socket_connection_in_Selector.patch, idleDisconnect.patch To reduce # socket connections, it would be useful for the new producer to close socket connections that are idle. We can introduce a new producer config for the idle time. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (KAFKA-1282) Disconnect idle socket connection in Selector
[ https://issues.apache.org/jira/browse/KAFKA-1282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14117638#comment-14117638 ] nicu marasoiu edited comment on KAFKA-1282 at 9/1/14 8:25 PM: -- I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. But I cannot compile test anymore! :core:classes :core:compileTestJava UP-TO-DATE :core:compileTestScala :core:processTestResources :core:testClasses Building :core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder, it happens when i checkout an old revision - seems to be something with gradle or scala changed in my local environment, although i did not make any explicit update or upgrade; i use Intellij). was (Author: nmarasoi): I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. Since the last rebase, I cannot compile test anymore! Building 0% :test_core_2_10_1 :kafka:core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder, it happens when i checkout an old revision - seems to be something with gradle or scala changed in my local environment, although i did not make any explicit update or upgrade; i use Intellij). Disconnect idle socket connection in Selector - Key: KAFKA-1282 URL: https://issues.apache.org/jira/browse/KAFKA-1282 Project: Kafka Issue Type: Bug Components: producer Affects Versions: 0.8.2 Reporter: Jun Rao Assignee: Neha Narkhede Labels: newbie++ Fix For: 0.9.0 Attachments: KAFKA-1282_Disconnect_idle_socket_connection_in_Selector.patch, idleDisconnect.patch To reduce # socket connections, it would be useful for the new producer to close socket connections that are idle. We can introduce a new producer config for the idle time. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (KAFKA-1282) Disconnect idle socket connection in Selector
[ https://issues.apache.org/jira/browse/KAFKA-1282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14117638#comment-14117638 ] nicu marasoiu edited comment on KAFKA-1282 at 9/1/14 8:28 PM: -- I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. But I cannot run the build anymore, it gets stuck at the phase below, right after compiling scala tests, the cpu goes to 0 and it simply hangs! :core:classes :core:compileTestJava UP-TO-DATE :core:compileTestScala :core:processTestResources :core:testClasses Building :core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder, it happens when i checkout an old revision - seems to be something with gradle or scala changed in my local environment, although i did not make any explicit update or upgrade; i use Intellij; git status clean, lsof shows no one is having open files in kafka src). was (Author: nmarasoi): I am sorry, Yes, that was the intent! I will write unit tests from now on to avoid such slips. Moreover, the removeEldestEntry will return false all the time, because it keeps the responsability of mutating the map for itself, as part of calling the close method. Attached the patch. But I cannot compile test anymore! :core:classes :core:compileTestJava UP-TO-DATE :core:compileTestScala :core:processTestResources :core:testClasses Building :core:test It remains stuck in this phase (and it happens with any scala version, it happens after disabling avast antivirus, and it happens after fully recloning the git in an antivirus excluded folder, it happens when i checkout an old revision - seems to be something with gradle or scala changed in my local environment, although i did not make any explicit update or upgrade; i use Intellij; git status clean, lsof shows no one is having open files in kafka src). Disconnect idle socket connection in Selector - Key: KAFKA-1282 URL: https://issues.apache.org/jira/browse/KAFKA-1282 Project: Kafka Issue Type: Bug Components: producer Affects Versions: 0.8.2 Reporter: Jun Rao Assignee: Neha Narkhede Labels: newbie++ Fix For: 0.9.0 Attachments: KAFKA-1282_Disconnect_idle_socket_connection_in_Selector.patch, idleDisconnect.patch To reduce # socket connections, it would be useful for the new producer to close socket connections that are idle. We can introduce a new producer config for the idle time. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 25155: Fix KAFKA-1616
On Aug. 28, 2014, 11:44 p.m., Jun Rao wrote: core/src/main/scala/kafka/server/RequestPurgatory.scala, line 258 https://reviews.apache.org/r/25155/diff/2/?file=671427#file671427line258 I am wonder if we should do two separate tests: (1) if enqueued() = purgeInterval, we purge the dealyed queue, (2) if size = purgeInterval, we purge the watchers. Added one test in RequestPurgatoryTest, which tests the number of watched elements and the enqueued requests before and after the purging. - Guozhang --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25155/#review51850 --- On Aug. 28, 2014, 5:12 p.m., Guozhang Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25155/ --- (Updated Aug. 28, 2014, 5:12 p.m.) Review request for kafka. Bugs: KAFKA-1616 https://issues.apache.org/jira/browse/KAFKA-1616 Repository: kafka Description --- Purgatory size to be the sum of watched list sizes; delayed request to be the expiry queue length; remove atomic integers for metrics Diffs - core/src/main/scala/kafka/server/RequestPurgatory.scala ce06d2c381348deef8559374869fcaed923da1d1 Diff: https://reviews.apache.org/r/25155/diff/ Testing --- Thanks, Guozhang Wang
Re: Review Request 24676: Fix KAFKA-1583
On Aug. 29, 2014, 1:42 a.m., Jun Rao wrote: core/src/main/scala/kafka/server/ReplicaManager.scala, line 725 https://reviews.apache.org/r/24676/diff/3-4/?file=666252#file666252line725 Do we need to add the new parameter? Does it hurt to write the checkpoint file in unit tests? The reason is that in some unit tests with Mock, checkoutpoint function will try to access Log.dir(), which is not easy to be mocked. On Aug. 29, 2014, 1:42 a.m., Jun Rao wrote: core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala, lines 171-172 https://reviews.apache.org/r/24676/diff/3-4/?file=666256#file666256line171 What it be better to return the correct offset and just return empty MessageSet? The equality test can be on the offset. The LogOffsetMetadata is only for the fetching start offset, which would be 0 for both cases; we need to make sure that the fetched data's end offset does not exceed the limit. On Aug. 29, 2014, 1:42 a.m., Jun Rao wrote: core/src/main/scala/kafka/server/KafkaApis.scala, line 169 https://reviews.apache.org/r/24676/diff/4/?file=670284#file670284line169 Yes, I think this should be debug level logging. I will fix on other places for handling requests. - Guozhang --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24676/#review51855 --- On Aug. 27, 2014, 5 p.m., Guozhang Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24676/ --- (Updated Aug. 27, 2014, 5 p.m.) Review request for kafka. Bugs: KAFKA-1583 https://issues.apache.org/jira/browse/KAFKA-1583 Repository: kafka Description --- Incorporated Jun's comments. 1. I left some cases in Log since they are return values for some of their APIs. 2. I kept the fetch info in the delayed fetch metadata since it needs to be used for re-reading the log. 3. I kept the name of callbackOnComplete by following the principle that only the caller knows what the callback is used for, and hence they can name the callback as reponseCallback (from KafkaApi) and putCacheCallback (from OffsetManager), all the callee will take the callback as callbackOnComplete. Unit test passed, with some other notes: 1. Found and fix a bug in the current delayed fetch satisifaction logic: previously when we calculate the bytes, we do not take in the fetchMaxBytes into consideration as an upper limit for a single partition's log, but simply get the diff between the current HW/LEO and the fetch offset. 2. Fount and fix a bug in the unit tests: we used to create replica manager on the fly but did not shut it down upon completing the test, which will leak the background thread (i.e. reaper thread of purgatory). 3. Changed the RequestPurgatory API a bit with Jun's comments. Now it has two implemented functions: forceComplete() and isCompleted(), and two functions that need to be instantiated in the subclasses: tryComplete() and complete(). Please let me know if people have more comments on the current API. 4. Cleaned the SimpleFetch test, previously this test is too complicate but it actually just test a simple logic of the replica manager. One concern I have now is about the online creation of a new callback function (i.e. the def inside the handling functions and offset manager's storeOffset function, when I am running the unit test with the patch it seems causing a higher CPU consumption than trunk). And could some one take a another pair of eyes in running the unit tests and check the CPU performance? Diffs - core/src/main/scala/kafka/api/FetchRequest.scala 51cdccf7f90eb530cc62b094ed822b8469d50b12 core/src/main/scala/kafka/api/FetchResponse.scala af9308737bf7832eca018c2b3ede703f7d1209f1 core/src/main/scala/kafka/api/OffsetCommitRequest.scala 861a6cf11dc6b6431fcbbe9de00c74a122f204bd core/src/main/scala/kafka/api/ProducerRequest.scala b2366e7eedcac17f657271d5293ff0bef6f3cbe6 core/src/main/scala/kafka/api/ProducerResponse.scala a286272c834b6f40164999ff8b7f8998875f2cfe core/src/main/scala/kafka/cluster/Partition.scala ff106b47e6ee194cea1cf589474fef975b9dd7e2 core/src/main/scala/kafka/common/ErrorMapping.scala 3fae7910e4ce17bc8325887a046f383e0c151d44 core/src/main/scala/kafka/log/Log.scala 0ddf97bd30311b6039e19abade41d2fbbad2f59b core/src/main/scala/kafka/network/BoundedByteBufferSend.scala a624359fb2059340bb8dc1619c5b5f226e26eb9b core/src/main/scala/kafka/server/DelayedFetch.scala e0f14e25af03e6d4344386dcabc1457ee784d345 core/src/main/scala/kafka/server/DelayedProduce.scala 9481508fc2d6140b36829840c337e557f3d090da
Re: Review Request 24676: Fix KAFKA-1583
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24676/ --- (Updated Sept. 2, 2014, 1:07 a.m.) Review request for kafka. Bugs: KAFKA-1583 https://issues.apache.org/jira/browse/KAFKA-1583 Repository: kafka Description (updated) --- TBD Diffs (updated) - core/src/main/scala/kafka/api/FetchRequest.scala 51cdccf7f90eb530cc62b094ed822b8469d50b12 core/src/main/scala/kafka/api/FetchResponse.scala af9308737bf7832eca018c2b3ede703f7d1209f1 core/src/main/scala/kafka/api/OffsetCommitRequest.scala 861a6cf11dc6b6431fcbbe9de00c74a122f204bd core/src/main/scala/kafka/api/ProducerRequest.scala b2366e7eedcac17f657271d5293ff0bef6f3cbe6 core/src/main/scala/kafka/api/ProducerResponse.scala a286272c834b6f40164999ff8b7f8998875f2cfe core/src/main/scala/kafka/cluster/Partition.scala ff106b47e6ee194cea1cf589474fef975b9dd7e2 core/src/main/scala/kafka/common/ErrorMapping.scala 3fae7910e4ce17bc8325887a046f383e0c151d44 core/src/main/scala/kafka/log/Log.scala 0ddf97bd30311b6039e19abade41d2fbbad2f59b core/src/main/scala/kafka/network/BoundedByteBufferSend.scala a624359fb2059340bb8dc1619c5b5f226e26eb9b core/src/main/scala/kafka/server/DelayedFetch.scala e0f14e25af03e6d4344386dcabc1457ee784d345 core/src/main/scala/kafka/server/DelayedProduce.scala 9481508fc2d6140b36829840c337e557f3d090da core/src/main/scala/kafka/server/FetchRequestPurgatory.scala ed1318891253556cdf4d908033b704495acd5724 core/src/main/scala/kafka/server/KafkaApis.scala c584b559416b3ee4bcbec5966be4891e0a03eefb core/src/main/scala/kafka/server/OffsetManager.scala 43eb2a35bb54d32c66cdb94772df657b3a104d1a core/src/main/scala/kafka/server/ProducerRequestPurgatory.scala d4a7d4a79b44263a1f7e1a92874dd36aa06e5a3f core/src/main/scala/kafka/server/ReplicaManager.scala 68758e35d496a4659819960ae8e809d6e215568e core/src/main/scala/kafka/server/RequestPurgatory.scala ce06d2c381348deef8559374869fcaed923da1d1 core/src/main/scala/kafka/utils/DelayedItem.scala d7276494072f14f1cdf7d23f755ac32678c5675c core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 03a424d45215e1e7780567d9559dae4d0ae6fc29 core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala cd302aa51eb8377d88b752d48274e403926439f2 core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala a9c4ddc78df0b3695a77a12cf8cf25521a203122 core/src/test/scala/unit/kafka/server/RequestPurgatoryTest.scala 168712de241125982d556c188c76514fceb93779 core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ab60e9b3a4d063c838bdc7f97b3ac7d2ede87072 core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 09ed8f5a7a414ae139803bf82d336c2d80bf4ac5 Diff: https://reviews.apache.org/r/24676/diff/ Testing --- Unit tests Thanks, Guozhang Wang
[jira] [Updated] (KAFKA-1583) Kafka API Refactoring
[ https://issues.apache.org/jira/browse/KAFKA-1583?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Guozhang Wang updated KAFKA-1583: - Attachment: KAFKA-1583_2014-09-01_18:07:42.patch Kafka API Refactoring - Key: KAFKA-1583 URL: https://issues.apache.org/jira/browse/KAFKA-1583 Project: Kafka Issue Type: Bug Reporter: Guozhang Wang Assignee: Guozhang Wang Fix For: 0.9.0 Attachments: KAFKA-1583.patch, KAFKA-1583_2014-08-20_13:54:38.patch, KAFKA-1583_2014-08-21_11:30:34.patch, KAFKA-1583_2014-08-27_09:44:50.patch, KAFKA-1583_2014-09-01_18:07:42.patch This is the next step of KAFKA-1430. Details can be found at this page: https://cwiki.apache.org/confluence/display/KAFKA/Kafka+API+Refactoring -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-1583) Kafka API Refactoring
[ https://issues.apache.org/jira/browse/KAFKA-1583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14117812#comment-14117812 ] Guozhang Wang commented on KAFKA-1583: -- Updated reviewboard https://reviews.apache.org/r/24676/diff/ against branch origin/trunk Kafka API Refactoring - Key: KAFKA-1583 URL: https://issues.apache.org/jira/browse/KAFKA-1583 Project: Kafka Issue Type: Bug Reporter: Guozhang Wang Assignee: Guozhang Wang Fix For: 0.9.0 Attachments: KAFKA-1583.patch, KAFKA-1583_2014-08-20_13:54:38.patch, KAFKA-1583_2014-08-21_11:30:34.patch, KAFKA-1583_2014-08-27_09:44:50.patch, KAFKA-1583_2014-09-01_18:07:42.patch This is the next step of KAFKA-1430. Details can be found at this page: https://cwiki.apache.org/confluence/display/KAFKA/Kafka+API+Refactoring -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: Review Request 24676: Fix KAFKA-1583
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24676/ --- (Updated Sept. 2, 2014, 1:09 a.m.) Review request for kafka. Bugs: KAFKA-1583 https://issues.apache.org/jira/browse/KAFKA-1583 Repository: kafka Description (updated) --- Incorporated Jun's comments round two. Incorporated Jun's comments. 1. I left some cases in Log since they are return values for some of their APIs. 2. I kept the fetch info in the delayed fetch metadata since it needs to be used for re-reading the log. 3. I kept the name of callbackOnComplete by following the principle that only the caller knows what the callback is used for, and hence they can name the callback as reponseCallback (from KafkaApi) and putCacheCallback (from OffsetManager), all the callee will take the callback as callbackOnComplete. Unit test passed, with some other notes: 1. Found and fix a bug in the current delayed fetch satisifaction logic: previously when we calculate the bytes, we do not take in the fetchMaxBytes into consideration as an upper limit for a single partition's log, but simply get the diff between the current HW/LEO and the fetch offset. 2. Found and fix a bug in the unit tests: we used to create replica manager on the fly but did not shut it down upon completing the test, which will leak the background thread (i.e. reaper thread of purgatory). 3. Changed the RequestPurgatory API a bit with Jun's comments. Now it has two implemented functions: forceComplete() and isCompleted(), and two functions that need to be instantiated in the subclasses: tryComplete() and complete(). Please let me know if people have more comments on the current API. 4. Cleaned the SimpleFetch test, previously this test is too complicate but it actually just test a simple logic of the replica manager. Diffs - core/src/main/scala/kafka/api/FetchRequest.scala 51cdccf7f90eb530cc62b094ed822b8469d50b12 core/src/main/scala/kafka/api/FetchResponse.scala af9308737bf7832eca018c2b3ede703f7d1209f1 core/src/main/scala/kafka/api/OffsetCommitRequest.scala 861a6cf11dc6b6431fcbbe9de00c74a122f204bd core/src/main/scala/kafka/api/ProducerRequest.scala b2366e7eedcac17f657271d5293ff0bef6f3cbe6 core/src/main/scala/kafka/api/ProducerResponse.scala a286272c834b6f40164999ff8b7f8998875f2cfe core/src/main/scala/kafka/cluster/Partition.scala ff106b47e6ee194cea1cf589474fef975b9dd7e2 core/src/main/scala/kafka/common/ErrorMapping.scala 3fae7910e4ce17bc8325887a046f383e0c151d44 core/src/main/scala/kafka/log/Log.scala 0ddf97bd30311b6039e19abade41d2fbbad2f59b core/src/main/scala/kafka/network/BoundedByteBufferSend.scala a624359fb2059340bb8dc1619c5b5f226e26eb9b core/src/main/scala/kafka/server/DelayedFetch.scala e0f14e25af03e6d4344386dcabc1457ee784d345 core/src/main/scala/kafka/server/DelayedProduce.scala 9481508fc2d6140b36829840c337e557f3d090da core/src/main/scala/kafka/server/FetchRequestPurgatory.scala ed1318891253556cdf4d908033b704495acd5724 core/src/main/scala/kafka/server/KafkaApis.scala c584b559416b3ee4bcbec5966be4891e0a03eefb core/src/main/scala/kafka/server/OffsetManager.scala 43eb2a35bb54d32c66cdb94772df657b3a104d1a core/src/main/scala/kafka/server/ProducerRequestPurgatory.scala d4a7d4a79b44263a1f7e1a92874dd36aa06e5a3f core/src/main/scala/kafka/server/ReplicaManager.scala 68758e35d496a4659819960ae8e809d6e215568e core/src/main/scala/kafka/server/RequestPurgatory.scala ce06d2c381348deef8559374869fcaed923da1d1 core/src/main/scala/kafka/utils/DelayedItem.scala d7276494072f14f1cdf7d23f755ac32678c5675c core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 03a424d45215e1e7780567d9559dae4d0ae6fc29 core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala cd302aa51eb8377d88b752d48274e403926439f2 core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala a9c4ddc78df0b3695a77a12cf8cf25521a203122 core/src/test/scala/unit/kafka/server/RequestPurgatoryTest.scala 168712de241125982d556c188c76514fceb93779 core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ab60e9b3a4d063c838bdc7f97b3ac7d2ede87072 core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 09ed8f5a7a414ae139803bf82d336c2d80bf4ac5 Diff: https://reviews.apache.org/r/24676/diff/ Testing --- Unit tests Thanks, Guozhang Wang
