[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16202256#comment-16202256
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/709


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16193283#comment-16193283
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
All tests pass with `docker/build.sh -t -n -i`

VOTE +1


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16191281#comment-16191281
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
Looks good.
VOTE +1


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16191206#comment-16191206
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
@BrynCooke i've been looking at this on and off since it was submitted and 
i can't come up with any concrete reason not to vote to merge this. i think we 
still get to release the {{evaluationFuture}} for GC with this added blob of 
code that handles cancellation of the cancellation. any last words?


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165813#comment-16165813
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user ko-mueller commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
Alright, it just would be nice to have this fixed in the next release. I 
didn't upgrade from 3.2.5 to 3.3.0 because of this. It's not a big issue for me 
right now, just that I'd like to stay current (without implementing a 
work-around for this).


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16164674#comment-16164674
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
@ko-mueller thanks for submitting this. I just wanted to let you know that 
it may take some time to get this merged. Every time this area of code gets 
changed there is suffering, so I'd like to be sure adding this code back 
doesn't create some new subtle issue that we solved with TINKERPOP-1714.


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16161393#comment-16161393
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

Github user dkuppitz commented on the issue:

https://github.com/apache/tinkerpop/pull/709
  
VOTE: +1


> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TINKERPOP-1761) GremlinExecutor: Timeout future not cancelled on successful script evaluation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16161030#comment-16161030
 ] 

ASF GitHub Bot commented on TINKERPOP-1761:
---

GitHub user ko-mueller opened a pull request:

https://github.com/apache/tinkerpop/pull/709

TINKERPOP-1761: Cancel script evaluation timeout

https://issues.apache.org/jira/browse/TINKERPOP-1761

I basically added the code again which was wrongly removed in 
TINKERPOP-1714, plus added a test. All unit tests passed and I confirmed 
manually that this fixes the issue I experienced.

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

$ git pull https://github.com/ko-mueller/tinkerpop TINKERPOP-1761

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

https://github.com/apache/tinkerpop/pull/709.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 #709


commit 2a8c92f245aa28b66c616e62230bfd3972e7e5b9
Author: Konstantin Mueller 
Date:   2017-09-11T09:25:12Z

TINKERPOP-1761: Cancel script evaluation timeout in  when script evaluation 
finished.




> GremlinExecutor: Timeout future not cancelled on successful script evaluation
> -
>
> Key: TINKERPOP-1761
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1761
> Project: TinkerPop
>  Issue Type: Bug
>  Components: groovy
>Affects Versions: 3.2.6
>Reporter: Konstantin Müller
>
> This bug seems to be introduced by TINKERPOP-1714. The timeout future in 
> GremlinExecutor.eval() is not cancelled when the evaluation future is 
> completed successfully before the timeout (lines 304ff). This was done 
> correctly before the changes introduced by TINKERPOP-1714. Code to reproduce:
> {code:java}
> try (GremlinExecutor executor = 
> GremlinExecutor.build().scriptEvaluationTimeout(15_000).create()) {
>   executor.eval("1+1").get();
> } catch (Exception e) {
>   // NOOP
> }
> {code}
> When the code leaves the try-block it calls GremlinExecutor.close() which 
> will clean up executorService and scheduledExecutorService, but because the 
> timeout future is not cancelled this will hang until the future is finished 
> (here 15s).
> Workaround: Provide an own ExecutorService when constructing a 
> GremlinExecutor instance and cancel all tasks when the script evaluation is 
> finished successfully.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)