You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by ogokal <gi...@git.apache.org> on 2015/08/30 20:08:49 UTC

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

GitHub user ogokal opened a pull request:

    https://github.com/apache/flink/pull/1077

    [FLINK-2125][streaming] Delimiter change from char to string

    I tried to change based on the previous comments. I hope it is sufficient enough.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ogokal/flink master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1077.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1077
    
----
commit c78b6d726c60b1e197bf5ee513e081c852362919
Author: ogokal <og...@gmail.com>
Date:   2015-08-30T17:42:56Z

    delimiter change from char to string

commit a51c486370e3e168912cbb71bde325701112d14b
Author: ogokal <og...@gmail.com>
Date:   2015-08-30T18:01:57Z

    [FLINK-2125][streaming] Delimiter change from char to string

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-139106600
  
    Hi, sorry for reply late.
    The file `SocketTextStreamFunction.java` has been changed recently.
    Can you update the code and squash your commits into one for the CI re-running?
    Since there was an issue in travis before.
    And generally this is good from my side.
    Maybe you need to @somecommiter to take a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-148333755
  
    OK, I will close this PR for now. 
    Please reopen if you would like to continue to work on this issue.
    Thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1077#discussion_r38282466
  
    --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/SocketTextStreamFunctionTest.java ---
    @@ -0,0 +1,237 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.streaming.api.functions;
    +
    +import org.apache.flink.streaming.api.functions.source.SocketTextStreamFunction;
    +import org.junit.Test;
    +
    +import java.io.ByteArrayInputStream;
    +import java.lang.reflect.Field;
    +import java.net.Socket;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +import static org.mockito.Mockito.mock;
    +import static org.mockito.Mockito.when;
    +
    +
    +public class SocketTextStreamFunctionTest {
    +    //Actual text
    +    /*
    +    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.
    +
    +    Integer aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. Praesent et felis tempor, laoreet libero eget, consequat nisl. Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.
    +
    +    Quisque sodales dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. Phasellus a metus dignissim risus auctor lacinia. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet facilisis.
    +
    +    Ut vitae volutpat odio. Sed eget vestibulum libero, eu tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.
    +
    +    In ac imperdiet ex, nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.
    +    */
    +    // Generated 5 paragraphs, 290 words, 2000 bytes of Lorem Ipsum
    +
    +    private static final String content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. " +
    +            "Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. " +
    +            "Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, " +
    +            "porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, " +
    +            "et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. " +
    +            "Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.\r\nInteger " +
    +            "aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac " +
    +            "mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. " +
    +            "Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam " +
    +            "eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. " +
    +            "Praesent et felis tempor, laoreet libero eget, consequat nisl. " +
    +            "Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.\r\nQuisque sodales " +
    +            "dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. " +
    +            "Phasellus a metus dignissim risus auctor lacinia. Class " +
    +            "aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos" +
    +            ". Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, " +
    +            "non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet " +
    +            "facilisis.\r\nUt vitae volutpat odio. Sed eget vestibulum libero, eu " +
    +            "tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, " +
    +            "et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam " +
    +            "euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. " +
    +            "Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. " +
    +            "Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. " +
    +            "Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.\r\nIn ac imperdiet ex, " +
    +            "nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.";
    +    private static final byte[] data = content.getBytes();
    +
    +    @Test
    +    public void testNewLineDelimitedOldApiWithChar() throws Exception {
    +        List<String> actualList = new ArrayList<>();
    +
    +        Socket socket = mock(Socket.class);
    +        when(socket.getInputStream()).thenReturn(new ByteArrayInputStream(data));
    +        when(socket.isClosed()).thenReturn(false);
    +        when(socket.isConnected()).thenReturn(true);
    +
    +        SocketTextStreamFunction source = new SocketTextStreamFunction("", 0, '\n', 0);
    +        Field field = SocketTextStreamFunction.class.getDeclaredField("isRunning");
    +        field.setAccessible(true);
    +        field.set(source, true);
    --- End diff --
    
    I do not think it is good to use reflection since I did something the same as you before and they told don`t do this.
    Why not set up a socket server in your test?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-139253154
  
    @ogokal, please ping me when you have rebased and updated the PR and I will have a look.
    Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-145855031
  
    Hi @ogokal, do you plan to update this PR? If not, could you please close it? Thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1077#discussion_r39749641
  
    --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunction.java ---
    @@ -117,12 +122,13 @@ private void streamFromSocket(SourceContext<String> ctx, Socket socket) throws E
     					continue;
     				}
     
    -				if (data == delimiter) {
    -					ctx.collect(buffer.toString());
    -					buffer = new StringBuilder();
    -				} else if (data != '\r') { // ignore carriage return
    -					buffer.append((char) data);
    +				buffer.append(charBuffer, 0, readCount);
    +				String[] splits = buffer.toString().split(delimiter);
    --- End diff --
    
    `String.split()` and `String.replace()` create new String objects which must be garbage collected. This adds quite some overhead, because these functions are called very often. The previous implementation was operating on a byte-level and avoiding the creation of new objects. It would be good if we could preserve this behavior.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1077#discussion_r38378115
  
    --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/SocketTextStreamFunctionTest.java ---
    @@ -0,0 +1,214 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.streaming.api.functions;
    +
    +import org.apache.flink.configuration.Configuration;
    +import org.apache.flink.streaming.api.functions.source.SocketTextStreamFunction;
    +import org.junit.After;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +import java.io.IOException;
    +import java.io.PrintWriter;
    +import java.net.ServerSocket;
    +import java.net.Socket;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +
    +public class SocketTextStreamFunctionTest {
    +    //Actual text
    +    /*
    +    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.
    +
    +    Integer aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. Praesent et felis tempor, laoreet libero eget, consequat nisl. Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.
    +
    +    Quisque sodales dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. Phasellus a metus dignissim risus auctor lacinia. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet facilisis.
    +
    +    Ut vitae volutpat odio. Sed eget vestibulum libero, eu tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.
    +
    +    In ac imperdiet ex, nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.
    +    */
    +    // Generated 5 paragraphs, 290 words, 2000 bytes of Lorem Ipsum
    +
    +    private static final String content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. " +
    +            "Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. " +
    +            "Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, " +
    +            "porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, " +
    +            "et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. " +
    +            "Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.\r\nInteger " +
    +            "aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac " +
    +            "mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. " +
    +            "Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam " +
    +            "eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. " +
    +            "Praesent et felis tempor, laoreet libero eget, consequat nisl. " +
    +            "Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.\r\nQuisque sodales " +
    +            "dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. " +
    +            "Phasellus a metus dignissim risus auctor lacinia. Class " +
    +            "aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos" +
    +            ". Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, " +
    +            "non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet " +
    +            "facilisis.\r\nUt vitae volutpat odio. Sed eget vestibulum libero, eu " +
    +            "tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, " +
    +            "et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam " +
    +            "euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. " +
    +            "Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. " +
    +            "Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. " +
    +            "Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.\r\nIn ac imperdiet ex, " +
    +            "nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.";
    +
    +    private static final String[] fakeServerResponses = content.split("\\.");
    +
    +    private class TestServer implements Runnable{
    +        private int port = 44444;
    --- End diff --
    
    It is not suitable to use a static port since sometimes it will get failed if this port is already used.
    You can call serverSocket = new ServerSocket(0); to set up a socket server with a free port.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1077#discussion_r38378250
  
    --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/SocketTextStreamFunctionTest.java ---
    @@ -0,0 +1,214 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.streaming.api.functions;
    +
    +import org.apache.flink.configuration.Configuration;
    +import org.apache.flink.streaming.api.functions.source.SocketTextStreamFunction;
    +import org.junit.After;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +import java.io.IOException;
    +import java.io.PrintWriter;
    +import java.net.ServerSocket;
    +import java.net.Socket;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +
    +public class SocketTextStreamFunctionTest {
    +    //Actual text
    +    /*
    +    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.
    +
    +    Integer aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. Praesent et felis tempor, laoreet libero eget, consequat nisl. Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.
    +
    +    Quisque sodales dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. Phasellus a metus dignissim risus auctor lacinia. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet facilisis.
    +
    +    Ut vitae volutpat odio. Sed eget vestibulum libero, eu tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.
    +
    +    In ac imperdiet ex, nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.
    +    */
    +    // Generated 5 paragraphs, 290 words, 2000 bytes of Lorem Ipsum
    +
    +    private static final String content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras sagittis nisl non euismod fermentum. " +
    +            "Curabitur lacinia vehicula enim quis tristique. Suspendisse imperdiet arcu sed bibendum vulputate. " +
    +            "Sed vitae nisl vitae turpis dapibus lacinia in id elit. Integer lorem dolor, " +
    +            "porttitor ut nisi in, tincidunt sodales leo. Aliquam tristique dui sit amet odio bibendum, " +
    +            "et rutrum turpis auctor. Morbi sit amet mollis augue, ac rutrum velit. " +
    +            "Vestibulum suscipit finibus sapien, et congue enim laoreet consequat.\r\nInteger " +
    +            "aliquam metus iaculis risus hendrerit maximus. Suspendisse vestibulum nibh ac " +
    +            "mauris cursus molestie sit amet vel turpis. Nulla et posuere orci. " +
    +            "Aliquam dui quam, posuere vitae erat vitae, finibus commodo ipsum. Aliquam " +
    +            "eu dui quis arcu porttitor sollicitudin. Integer sodales finibus ullamcorper. " +
    +            "Praesent et felis tempor, laoreet libero eget, consequat nisl. " +
    +            "Aenean molestie rutrum lorem, ac cursus nisl dapibus vitae.\r\nQuisque sodales " +
    +            "dui et sem bibendum semper. Pellentesque luctus leo nec lacus euismod pellentesque. " +
    +            "Phasellus a metus dignissim risus auctor lacinia. Class " +
    +            "aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos" +
    +            ". Aenean consectetur bibendum imperdiet. Etiam dignissim rutrum enim, " +
    +            "non volutpat nisi condimentum sed. Quisque condimentum ultrices est sit amet " +
    +            "facilisis.\r\nUt vitae volutpat odio. Sed eget vestibulum libero, eu " +
    +            "tincidunt lorem. Nam pretium nulla nisl. Maecenas fringilla nunc ut turpis consectetur, " +
    +            "et fringilla sem placerat. Etiam nec scelerisque nisi, at sodales ligula. Aliquam " +
    +            "euismod faucibus egestas. Curabitur eget enim quam. Praesent convallis mattis lobortis. " +
    +            "Pellentesque a consectetur nisl. Duis molestie diam est. Nam a malesuada augue. " +
    +            "Vivamus enim massa, luctus ac elit ut, vestibulum laoreet nulla. " +
    +            "Curabitur pellentesque vel mi eget tempus. Donec cursus et leo quis viverra.\r\nIn ac imperdiet ex, " +
    +            "nec aliquet erat. Nullam sit amet enim in dolor finibus convallis id eu nibh. Fusce aliquam convallis orci aliquam.";
    +
    +    private static final String[] fakeServerResponses = content.split("\\.");
    +
    +    private class TestServer implements Runnable{
    +        private int port = 44444;
    +        public int getPort(){
    +            return port;
    +        }
    +        private ServerSocket serverSocket;
    +
    +        @Override public void run() {
    +            try {
    +                serverSocket = new ServerSocket(port);
    +                while(true){
    +                    Socket clientSocket = serverSocket.accept();
    +                    try {
    +                        PrintWriter out = new PrintWriter(clientSocket.getOutputStream());
    +                        for (String res : fakeServerResponses){
    +                            out.print(res+".");
    +                            out.flush();
    +                        }
    +                    }finally {
    +                        clientSocket.close();
    +                    }
    +                }
    +            } catch (IOException e) {
    +            }
    +        }
    +
    +        public void stopServer() throws IOException {
    +            serverSocket.close();
    +        }
    +    }
    +
    +    private TestServer testServer;
    +
    +    @Before
    +    public void startTestServerSocket(){
    +        testServer = new TestServer();
    +        new Thread(testServer).start();
    +    }
    +
    +    @After
    +    public void stopTestServerSocket() throws IOException {
    +        testServer.stopServer();
    +    }
    +
    +    private void prepareTestForOld(char delimiter,List<String> actualList) throws Exception {
    +        SocketTextStreamFunction source = new SocketTextStreamFunction("localhost", testServer.getPort(), delimiter, 0);
    +        final ListSourceContext<String> flinkCollector = new ListSourceContext<String>(actualList);
    +        source.open(new Configuration());
    +        source.run(flinkCollector);
    +    }
    +
    +    private void prepareTest(String delimiter,List<String> actualList) throws Exception {
    +        SocketTextStreamFunction source = new SocketTextStreamFunction("localhost", testServer.getPort(), delimiter, 0);
    +        final ListSourceContext<String> flinkCollector = new ListSourceContext<String>(actualList);
    +        source.open(new Configuration());
    +        source.run(flinkCollector);
    +    }
    +
    +    @Test
    +    public void testNewLineDelimitedOldApiWithChar() throws Exception {
    +        List<String> actualList = new ArrayList<>();
    +        prepareTestForOld('\n',actualList);
    +        assertEquals(5, actualList.size());
    --- End diff --
    
    Can you assert the whole received buffer in SocketTextSource is correct?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1077#discussion_r39749738
  
    --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunction.java ---
    @@ -70,14 +74,15 @@ public void run(SourceContext<String> ctx) throws Exception {
     
     	private void streamFromSocket(SourceContext<String> ctx, Socket socket) throws Exception {
     		try {
    -			StringBuilder buffer = new StringBuilder();
    +			StringBuffer buffer = new StringBuffer();
    +			char[] charBuffer = new char[Math.max(8192, 2 * delimiter.length())];
     			BufferedReader reader = new BufferedReader(new InputStreamReader(
     					socket.getInputStream()));
     
     			while (isRunning) {
    -				int data;
    +				int readCount;
     				try {
    -					data = reader.read();
    +					readCount = reader.read(charBuffer);
    --- End diff --
    
    Good change. It's much better to read a buffer instead of individual characters.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by ogokal <gi...@git.apache.org>.
Github user ogokal commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-136486224
  
    I changed unit tests as HuangWHWHW suggested, added a simple test server socket and refactored test methods. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1077


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by ogokal <gi...@git.apache.org>.
Github user ogokal commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-136828049
  
    I changed static port and added assertions for full content 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:

    https://github.com/apache/flink/pull/1077#issuecomment-136243644
  
    
    Hi,
    Generally this is a good idea to receive all buffer once instead get a char every time.
    And you can see my PR:https://github.com/apache/flink/pull/992.
    There will be some changes in SocketTextStreamFunctionTest.java.
    And These changes is just "maybe".
    I am not sure since that PR has not been merged yet.
    Just provide you an info.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---