search

TestRoots / watchdog

IntelliJ & Eclipse plugin for monitoring how Java applications are developed and tested
http://www.testroots.org
Other
18 stars 10 forks source link

Reduce the shutdown time in case of a network error #222

Closed nspruit closed 8 years ago

nspruit commented 8 years ago

This PR reduces the connection timeout from 12 to 3 seconds before shutting down the IDE. In this way, Eclipse is shut down after 3 seconds instead of 12 in case of a very slow connection. The connection timeout that is used during the remainder of the lifetime of the plugin remains 12 seconds. This PR should therefore close #129.

Inventitech commented 8 years ago

Awesome, I really like this solution. A few comments inline.

Inventitech commented 8 years ago

One problem that still persists with this solution is if the transfer somehow takes very long. I think that it is unacceptable that we slow down the shutdown of Eclipse by more than 2s. Could you update the PR with such functionality?

nspruit commented 8 years ago

The first two comments are addressed in the first commit. The second commit contains a proposal for shutting down eclipse faster when a transfer takes very long.

The build should actually pass (as it does locally), maybe there is a timing dependency in the failed test? However, I can't restart the build without closing and reopening this PR.

Inventitech commented 8 years ago

A few smaller comments. Did you investigate what happens on the server when a transfer is killed mid-way?

nspruit commented 8 years ago

When the request has not completely been send when the connection is closed, I think nothing happens on the server, as the method post '/user/:uid/:pid/intervals' will probably not be executed before the request is received completely (as the request object does not exist). However, I'm not 100% sure, as I don't know what Sinatra does internally in this case. But I don't think that Sinatra continues with a request that hasn't been received completely.

In the other case where the request is received entirely, but the connection is closed before the response is sent to the client, the request will be executed. This means that that the plugin thinks it has not send the latest intervals to the server, while it has actually done so. This in turn means that the client will resend these intervals when it is started again. Therefore, these intervals will probably be entered in the DB twice.

Inventitech commented 8 years ago

Thanks for the PR!