You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/07/27 16:54:05 UTC

[GitHub] [tinkerpop] kenhuuu opened a new pull request, #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

kenhuuu opened a new pull request, #1768:
URL: https://github.com/apache/tinkerpop/pull/1768

   An issue occurs when the GremlinGroovyScriptEngine is called from multiple threads to evaluate bytecode that contains lambdas. The default translator is not thread-safe which results in queries being interlaced with one another. This is fixed by creating a new instance of the translator per translation so that the translation process isn't interfered with. This primarily affects the UnifiedChannelizer because it currently uses the same instance of the translator from multiple threads.
   
   Fixes https://issues.apache.org/jira/browse/TINKERPOP-2765


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] spmallette merged pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
spmallette merged PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] kenhuuu commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
kenhuuu commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1199841186

   > Do you feel like this is just a patch for the stated problem in the JIRA? Would it be better if the translator were just made to be thread-safe:
   > 
   > https://issues.apache.org/jira/browse/TINKERPOP-2675
   > 
   > I'm not against merging this as it is to solve the problem, but i'm curious about the bigger issue of general thread-safety and whether or not it would make sense to refactor as TINKERPOP-2675 suggests or if your opinion would be to leave that all as it is.
   
   This is something that I thought about when investigating the fix, but I decided on making this simple patch so that it could be merged into 3.5-dev. I think it does make more sense to refactor the translators but that would likely require a breaking change so I thought it wasn't something we would want to do for 3.5-dev.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] spmallette commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1199785863

   Do you feel like this is just a patch for the stated problem in the JIRA? Would it be better if the translator were just made to be thread-safe:
   
   https://issues.apache.org/jira/browse/TINKERPOP-2675
   
   I'm not against merging this as it is to solve the problem, but i'm curious about the bigger issue of general thread-safety and whether or not it would make sense to refactor as TINKERPOP-2675 suggests or if your opinion would be to leave that all as it is. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] spmallette commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1201099553

   Can you please add a CHANGELOG entry and submit a second PR for 3.6-dev (the merge will likely conflict given the heavy refactoring that took place between the two branches)? I suggest you do the CHANGELOG for this PR first, then I can merge this one through the branches. I'll ignore conflicts for 3.6-dev/master and then you can issue the second PR to 3.6-dev. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] spmallette commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1206719408

   >  I'll ignore conflicts for 3.6-dev/master and then you can issue the second PR to 3.6-dev.
   
   @kenhuuu could you please submit the PR against 3.6-dev for these changes as the merge is now complete:


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] codecov-commenter commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1197063914

   # [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1768?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1768](https://codecov.io/gh/apache/tinkerpop/pull/1768?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b471529) into [3.5-dev](https://codecov.io/gh/apache/tinkerpop/commit/137cad03e7994b6ba08a15307c2d3d80a9f26f75?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (137cad0) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   ```diff
   @@           Coverage Diff            @@
   ##           3.5-dev    #1768   +/-   ##
   ========================================
     Coverage    63.58%   63.58%           
   ========================================
     Files           23       23           
     Lines         3636     3636           
   ========================================
     Hits          2312     2312           
     Misses        1145     1145           
     Partials       179      179           
   ```
   
   
   
   Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] kenhuuu commented on pull request #1768: TINKERPOP-2765 Fix concurrency issue during script translation in the GremlinGroovyScriptEngine.

Posted by GitBox <gi...@apache.org>.
kenhuuu commented on PR #1768:
URL: https://github.com/apache/tinkerpop/pull/1768#issuecomment-1207089978

   Sure, I made a PR for 3.6-dev at https://github.com/apache/tinkerpop/pull/1778. Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org