Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

org.synbiohub.frontend.SynBioHubFrontend should include timeouts #559

Closed
jakebeal opened this issue Aug 15, 2018 · 10 comments
Closed

org.synbiohub.frontend.SynBioHubFrontend should include timeouts #559

jakebeal opened this issue Aug 15, 2018 · 10 comments
Assignees
Milestone

Comments

@jakebeal
Copy link
Contributor

Unless specifically disabled, org.synbiohub.frontend.SynBioHubFrontend should include an HTTP timeout in the configuration of the HTTP connection.

Currently when fetching items from SynBioHub, the SynBioHubFrontend class performs a blocking read inside of org.synbiohub.frontend.SynBioHubFrontend.fetchContentAsInputStream. Normally, this is not a problem, but when a read dies in the middle (as can happen with very large pulls on occasion), this leaves the thread hung with no exception.

This issue is currently causing freezes in the SD2 Dictionary Maintainer. It can be kludged around by implementing a timeout on SynBioHubFrontend, but should properly be addressed within the library by setting the timeout on the HTTP operation instead.

@jakebeal jakebeal added the bug label Aug 15, 2018
@jakebeal
Copy link
Contributor Author

Additional diagnostic information. A read that normally completes in a couple of minutes hung for multiple hours. Stack inspection found the following trace, identifying the issue:

"main" #1 prio=5 os_prio=0 tid=0x00007f5e8400a800 nid=0x1e4 runnable [0x00007f5e8cb9e000]
   java.lang.Thread.State: RUNNABLE
        at java.net.SocketInputStream.socketRead0(Native Method)
        at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
        at java.net.SocketInputStream.read(SocketInputStream.java:171)
        at java.net.SocketInputStream.read(SocketInputStream.java:141)
        at sun.security.ssl.InputRecord.readFully(InputRecord.java:465)
        at sun.security.ssl.InputRecord.read(InputRecord.java:503)
        at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
        - locked <0x00000005eb58cb98> (a java.lang.Object)
        at sun.security.ssl.SSLSocketImpl.readDataRecord(SSLSocketImpl.java:940)
        at sun.security.ssl.AppInputStream.read(AppInputStream.java:105)
        - locked <0x00000005eb58bdd8> (a sun.security.ssl.AppInputStream)
        at org.apache.http.impl.io.SessionInputBufferImpl.streamRead(SessionInputBufferImpl.java:137)
        at org.apache.http.impl.io.SessionInputBufferImpl.fillBuffer(SessionInputBufferImpl.java:153)
        at org.apache.http.impl.io.SessionInputBufferImpl.readLine(SessionInputBufferImpl.java:282)
        at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:138)
        at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:56)
        at org.apache.http.impl.io.AbstractMessageParser.parse(AbstractMessageParser.java:259)
        at org.apache.http.impl.DefaultBHttpClientConnection.receiveResponseHeader(DefaultBHttpClientConnection.java:163)
        at org.apache.http.impl.conn.CPoolProxy.receiveResponseHeader(CPoolProxy.java:165)
        at org.apache.http.protocol.HttpRequestExecutor.doReceiveResponse(HttpRequestExecutor.java:273)
        at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:125)
        at org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:272)
        at org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:185)
        at org.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:89)
        at org.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:111)
        at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:185)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:83)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:108)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:56)
        at org.synbiohub.frontend.SynBioHubFrontend.fetchContentAsInputStream(SynBioHubFrontend.java:1106)
        at org.synbiohub.frontend.SynBioHubFrontend.fetchFromSynBioHub(SynBioHubFrontend.java:978)
        at org.synbiohub.frontend.SynBioHubFrontend.getSBOL(SynBioHubFrontend.java:129)
        at com.bbn.sd2.SynBioHubAccessor.retrieve(SynBioHubAccessor.java:161)
        at com.bbn.sd2.MaintainDictionary.update_entry(MaintainDictionary.java:226)
        at com.bbn.sd2.MaintainDictionary.maintain_dictionary(MaintainDictionary.java:318)
        at com.bbn.sd2.DictionaryMaintainerApp.main(DictionaryMaintainerApp.java:31)

@cjmyers
Copy link
Contributor

cjmyers commented Aug 17, 2018

Discussing this with @zachzundel, he thinks we should implement the solution on the SynBioHub side. Namely, that the HTTP requests should initiate the request, and return as quickly as possible. SBH should cache the result of the fetch and the frontend would be able to repeat the request eventually getting the document back once the fetch has concluded. This will require changes on both SBH and libSBOLj, but it seems this will be less overhead and be more robust in the long run. @zachzundel please add your thoughts and also open the SBH issue for this too.

@cjmyers cjmyers added the change label Aug 17, 2018
@cjmyers cjmyers added this to the SBOL 2.4 milestone Aug 17, 2018
@jakebeal
Copy link
Contributor Author

Let me run two scenarios past you:

  1. I'm a user downloading a large file over a terrible connection, and I'm happy to let the download run all night while I am sleeping.
  2. I'm a tool downloading a large file as part of an automated batch process, and I want to either finish in 2 minutes or move on to the next.

I don't see any way for SynBioHub to distinguish between these two clients. Thus, I think that it's going to be important to let the client also be able to set their own abort. And if libSBOLj doesn't do it, then we're probably just going to write a library function around the library that will abort the library instead.

@cjmyers
Copy link
Contributor

cjmyers commented Aug 17, 2018

I think what we are suggesting will address your concerns. The idea is that SBH would not be blocking. Namely, you send a request then it would respond that it got your request. You could probe the same URL to see when the request has been completed. The library then could have a blocking method that essentially busy waits polling for the request to be processed, as well as another method which takes a time limit and polls until the time limit has been exceeded.

@jakebeal
Copy link
Contributor Author

That doesn't address the case of a very slow download.

@3ach
Copy link
Contributor

3ach commented Aug 17, 2018

I think we can still add a timeout to the SynBioHubFrontend, but SynBioHub itself will be more asynchronous, like Chris described.

@cjmyers
Copy link
Contributor

cjmyers commented Aug 17, 2018

Very slow download is usually due to the time it takes to create the SBOL file. Certainly, if we get SBOL files in the GBs, then we may see it to be a transfer problem, but right now it is usually not the bottleneck. Even so, I think a timeout that I propose will allow you to say how long you are willing to wait for the download, and this may be file construction and/or download time.

@cjmyers
Copy link
Contributor

cjmyers commented Sep 30, 2018 via email

@cjmyers cjmyers assigned jakebeal and unassigned 3ach and cjmyers Sep 30, 2018
@cjmyers cjmyers added needs testing and removed bug labels Sep 30, 2018
@jakebeal
Copy link
Contributor Author

@bbartley Can you please test this with the SD2 dictionary?

@jakebeal jakebeal assigned bbartley and unassigned jakebeal Sep 30, 2018
@jakebeal
Copy link
Contributor Author

jakebeal commented Mar 4, 2019

@dsumorok-raytheon has incorporated use of this in the SD2 dictionary, so I think it can be counted as tested and resolved.

@cjmyers cjmyers closed this as completed Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants