[GitHub] qpid-proton pull request: proton-j: SSL decode buffer implementati...

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: proton-j: SSL decode buffer implementati...

ted-ross
GitHub user SreeramGarlapati opened a pull request:

    https://github.com/apache/qpid-proton/pull/73

    proton-j: SSL decode buffer implementation doesn't flush all bytes to the next transport buffer (_underlyingInput)

    Actual Issue/scenario hit by Microsoft Azure EventHubs:
    We have a pattern where customers sends messages in a burst to our Queue and stop sending and then wait for all of them to be received. Because of this issue - many messages were stuck in the SSL Decode Buffer and were only picked up when new messages arrive.

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

    $ git pull https://github.com/SreeramGarlapati/qpid-proton sg.recvstuck

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

    https://github.com/apache/qpid-proton/pull/73.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 #73
   
----
commit 2ceb434e1720917e788f55f8439095cb1be0fb78
Author: Sreeram Garlapati <[hidden email]>
Date:   2016-04-05T04:34:01Z

    JavaClient: Fix stuck issue on Receiver
    Underlying Proton-J Issue: Messages were stuck in the SSL Decode Buffer and were only picked up when new messages arrive.

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: PROTON-1171: SimpleSSLTransportWrapper d...

ted-ross
Github user SreeramGarlapati commented on a diff in the pull request:

    https://github.com/apache/qpid-proton/pull/73#discussion_r59236240
 
    --- Diff: proton-j/src/test/java/org/apache/qpid/proton/engine/impl/ssl/SimpleSslTransportWrapperTest.java ---
    @@ -117,16 +117,6 @@ public void testInputIncompletePacketInThreeParts()
         }
     
         @Test
    -    public void testUnderlyingInputUsingSmallBuffer_receivesAllDecodedInput() throws Exception
    -    {
    -        _underlyingInput.setInputBufferSize(1);
    --- End diff --
   
    I deleted this Test from this File and Moved it to a brand new file for 2 reasons:
    1) This function has a bug - SetInputBufferSize - sets only the integer value. It doesn't change anything with actual Buffer Size.
    2) Since, changing buffer size will change the behavior of SimpleSslTransportWrapper - this could impact other testcases in the same file. So, moved the Size related test to a separate file...


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: PROTON-1171: SimpleSSLTransportWrapper d...

ted-ross
In reply to this post by ted-ross
Github user asfgit closed the pull request at:

    https://github.com/apache/qpid-proton/pull/73


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: PROTON-1171: SimpleSSLTransportWrapper d...

ted-ross
In reply to this post by ted-ross
Github user gemmellr commented on the pull request:

    https://github.com/apache/qpid-proton/pull/73#issuecomment-209455781
 
    I have made a change that should resolve this. Its a little different than your proposal to both resolve an issue with it (unlikely but possible infinite loop), and also change some other existing behaviours I found while digging into it. If you could give it a try with your bits and report back that would be great.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: PROTON-1171: SimpleSSLTransportWrapper d...

ted-ross
In reply to this post by ted-ross
Github user SreeramGarlapati commented on the pull request:

    https://github.com/apache/qpid-proton/pull/73#issuecomment-209605630
 
    This is huge. Thanks @gemmellr !
    We validated this in our stress run - you are correct - with my fix - we were still hitting another issue - which I was still looking at. After I packed proton with your commit - we are completely unblocked from receive stuck issue.
   
    Looking forward for 0.12.2.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] qpid-proton pull request: PROTON-1171: SimpleSSLTransportWrapper d...

ted-ross
In reply to this post by ted-ross
Github user SreeramGarlapati commented on the pull request:

    https://github.com/apache/qpid-proton/pull/73#issuecomment-210225444
 
    After a couple of hours of our Stress run - we are still experiencing some bytes stuck in this layer. Have sent you a mail on the proton forum. Thx!


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Loading...