You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/12/11 15:19:40 UTC
[GitHub] [kafka] tombentley opened a new pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
tombentley opened a new pull request #9735:
URL: https://github.com/apache/kafka/pull/9735
See [KAFKA-10846](https://issues.apache.org/jira/browse/KAFKA-10846)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#discussion_r545182111
##########
File path: connect/file/src/test/java/org/apache/kafka/connect/file/FileStreamSourceTaskTest.java
##########
@@ -77,6 +78,17 @@ private void replay() {
@Test
public void testNormalLifecycle() throws InterruptedException, IOException {
+ normalLifecycle();
+ }
+
+ @Test
+ public void testNormalLifecycleWithResize() throws InterruptedException, IOException {
+ task = new FileStreamSourceTask(2);
Review comment:
Or we make each test create task instead of creating task in ```setup```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-747524357
@chia7712 I've implemented the first option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-745997459
Thanks for taking a look @chia7712. I've added a test, but had to expose the buffer size.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-747954608
@chia7712 thank _you_ for the reviews.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 merged pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #9735:
URL: https://github.com/apache/kafka/pull/9735
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-747853413
@tombentley Thanks for this contribution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-746008036
@chia7712 I think `testNormalLifecycleWithResize()` covers that when a buffer resize is required, and `testNormalLifecycle()` in the case where a buffer resize is not required. Or did I miss a case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-746011953
> I think testNormalLifecycleWithResize() covers that when a buffer resize is required, and testNormalLifecycle() in the case where a buffer resize is not required. Or did I miss a case?
Sorry that I neglect ```testNormalLifecycleWithResize```. It is a little hard to review code by iPhone. Will take another look later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-745781492
@tombentley Thanks for this patch. Could you offer a test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#discussion_r545179120
##########
File path: connect/file/src/test/java/org/apache/kafka/connect/file/FileStreamSourceTaskTest.java
##########
@@ -77,6 +78,17 @@ private void replay() {
@Test
public void testNormalLifecycle() throws InterruptedException, IOException {
+ normalLifecycle();
+ }
+
+ @Test
+ public void testNormalLifecycleWithResize() throws InterruptedException, IOException {
+ task = new FileStreamSourceTask(2);
Review comment:
This new test case LGTM. However, it re-create the task. How about setting a small initial size of FileStreamSourceTask to make all test cases meet the resize?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9735: KAFKA-10846: Grow buffer in FileSourceStreamTask only when needed
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9735:
URL: https://github.com/apache/kafka/pull/9735#issuecomment-743254772
@C0urante @kkonstantine maybe one of you could take a look.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org