Skip to content

GremlinServer: Restrict the thread pool size to 1 when serving a TinkerTransactionGraph#2330

Closed
danielcweber wants to merge 1 commit intoapache:masterfrom
danielcweber:SerializeTinkerTransactionGraphRequests
Closed

GremlinServer: Restrict the thread pool size to 1 when serving a TinkerTransactionGraph#2330
danielcweber wants to merge 1 commit intoapache:masterfrom
danielcweber:SerializeTinkerTransactionGraphRequests

Conversation

@danielcweber
Copy link
Contributor

@danielcweber danielcweber commented Nov 7, 2023

Executing sessionless requests on different threads leads to unpredictable behaviour, even if requests are serialized on the client side (even with some reasonable wait time between requests). Wrapping each request in a transaction on the client side works just fine however, so this change might be a bit to broad for explicit transaction processing. Maybe there is a way to only restrict the sessionless processor and leave the session-processor multi-threaded?

…erTransactionGraph. For sessionless requests, executing requests on different threads leads to unpredictable behaviour, even if requests are serialized (even with some in-between wait time) on the client side.
@codecov-commenter
Copy link

Codecov Report

Merging #2330 (e49c600) into master (1e549f1) will decrease coverage by 0.04%.
Report is 12 commits behind head on master.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2330      +/-   ##
============================================
- Coverage     75.92%   75.88%   -0.04%     
+ Complexity    13018    13013       -5     
============================================
  Files          1082     1082              
  Lines         64902    64902              
  Branches       7239     7239              
============================================
- Hits          49275    49253      -22     
- Misses        12953    12983      +30     
+ Partials       2674     2666       -8     

see 8 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@vkagamlyk
Copy link
Contributor

Hi Daniel,
Could you share some details about unpredictable behavior? Maybe some code examples or steps to reproduce the problem

@danielcweber
Copy link
Contributor Author

danielcweber commented Nov 10, 2023

Maybe some code examples or steps to reproduce the problem

That's my main issue...I can't reliably.

Could you share some details about unpredictable behavior

We got a large set of integration tests interfacing Gremlin Server with "conf/gremlin-server-transaction.yaml" setup. The tests run sequentially. Even with a pool size of 1, and even with an artificial wait time between tests, the results from Gremlin Server are randomly missing vertex properties. It seems to be only vertex properties. They are added on one unit test, and are missing on the next. It all goes away when the server is configured to run on one thread only.

The main question here is: Is it expected that TinkerTransactionGraph in sessionless mode may exhibit non durable updates, even when requests are run sequentially, even with some wait time? It seems, as it is currently, that it's only to expected to work reliably when requests are run on the same thread each. If that is the case, my PR makes sense. If it's not the case, there might be a bug in TinkerTransactionGraph.

I will continue to try to reproduce this reliably.

@vkagamlyk
Copy link
Contributor

PR with fix #2343

@danielcweber
Copy link
Contributor Author

#2343 works for us. Thanks a lot!

@danielcweber danielcweber deleted the SerializeTinkerTransactionGraphRequests branch December 8, 2023 15:02
@danielcweber danielcweber restored the SerializeTinkerTransactionGraphRequests branch August 23, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants