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 2020/12/18 21:32:32 UTC

[GitHub] [tinkerpop] giovibal opened a new pull request #1374: Fix: add Vertex and Edge's properties in graphbinary serialization

giovibal opened a new pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374


   Add Vertex and Edge's properties in graphbinary serialization
   


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] giovibal edited a comment on pull request #1374: Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
giovibal edited a comment on pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374#issuecomment-748503564


   I need to execute more tests, because in my dev environment ( tinkerpop v3.4.6 + bitsy ), given a query like this: `g.V().has("filter",1)`, graphson and kryo send all properties, but graphbinary no.
   
   Anyway thank you for your patience, you've completely answered to my question :+1: 
   


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] giovibal commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
giovibal commented on pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374#issuecomment-748631478


   > How do you like Bitsy by the way (if you don't mind taking a moment to share your experience)?
   
   We use bitsy and gremlin-server to store configurations and schema of our IoT Cloud Platform.
   
   We manage it in production for about 3 years, 2 instances in cloud with kubernetes.
   
   Our graphs are "tenantized" with partition strategy, 
   and sizes are around of 10-20k edges / and 5-10k vertices for bigger partitions.
   
   The main goals we reach with Bitsy are:
   
   1. speed (in ram, so speed for little graphs is great)
   
   2. storage data format is easy readable and manipulation can be done with little effort. So we can mantain for long time also if need to change everithing or fix things.
   
   For now the only drawback is leak of a sort of high availability.
   
   Recently I'am working on a sort of "change-data-capture" throught a gremlin server plugin that intercept modifications and send json events throught kafka.
   
   I Confess: would like a rust implementation of gremlin server with clustering features.
   
   Hope to not got out of topic, thank for your kindness.


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] giovibal commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
giovibal commented on pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374#issuecomment-748892525


   > When I'd said that Gremlin Server had configured reference elements by default I meant that the out of the box server setup did that. If you configured Bitsy it's likely that you didn't add `ReferenceElementStrategy` to that setup. It also assumes you were likely using Java to connect to that server which is the only language that will support non-references.
   
   Found it: we use custom .groovy initialization with our custom strategies, our docker build overwrite default .groovy and this is our final line:
   
   ```
   globals << [g : graph.traversal()]
   ```
   
   instead of the standard from v3.4.6:
   
   ```
   globals << [g : graph.traversal().withStrategies(ReferenceElementStrategy.instance())]
   ```
   and yes: we use java client.


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] spmallette closed pull request #1374: Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
spmallette closed pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374


   


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] spmallette commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

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


   When I'd said that Gremlin Server had configured reference elements by default I meant that the out of the box server setup did that. If you configured Bitsy it's likely that you didn't add `ReferenceElementStrategy` to that setup.  It also assumes you were likely using Java to connect to that server which is the only language that will support non-references. So, there is a lot of if-thens-elses to this story I'm afraid, with a complex history behind it all, which is why we are where we are with all this. Sorry If that caused confusion and I hope we can make this all work more consistently in future versions. It's just going to take a fair bit of thought and discussion to get it right. 
   
   How do you like Bitsy by the way (if you don't mind taking a moment to share your experience)?


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] giovibal commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
giovibal commented on pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374#issuecomment-748503564


   I need to execute more tests, because in my dev environment ( tinkerpop v3.4.6 + bitsy ), given a query like this: `g.V().has("filter",1)`, graphson and kryo send all properties, but graphbinary no.
   
   Anyway thank you for your patience, you've completely answered to my question.
   


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] spmallette commented on pull request #1374: Fix: add Vertex and Edge's properties in graphbinary serialization

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


   Thank you for your contribution, but I'm afraid that this is not a bug to fix. It is a community design decision and effectively is how it is supposed to be. If you would like to familiarize yourself with the history of this decision as well as the ramifications of the change and all of the related issues that would need to be addressed as part of putting properties on graph elements I'd point you to this dev list thread:
   
   https://lists.apache.org/thread.html/e959e85d4f8b3d46d281f2742a6e574c7d27c54bfc52f802f7c04af3%40%3Cdev.tinkerpop.apache.org%3E
   
   Understanding that thread is crucial to having discussions on this topic. Take care!
   
   


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] giovibal commented on pull request #1374: Fix: add Vertex and Edge's properties in graphbinary serialization

Posted by GitBox <gi...@apache.org>.
giovibal commented on pull request #1374:
URL: https://github.com/apache/tinkerpop/pull/1374#issuecomment-748441730


   Thank you, I've read the history about references, and essentially this is the point that make me considered this a "bug": 
   
   > 5. GraphBinary has left a placeholder for properties in elements but is
   > natively built to not serialize properties at all. This approach of course
   > creates an asymmetry with other serialization formats like GraphSON and
   > Gryo as it pertains to IO.
   > 
   
   So, my question is: will properties disappears from GraphSON and Kryo serialization too ?


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] spmallette commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

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


   Ok - thanks for confirming what was happening. I'll leave you with one final thought before I close this, in general, even if we returned properties I tend to think that returning an entire vertex or edge is too close to `SELECT * FROM table` which isn't considered a good practice in the SQL world. The same sorts of problems can apply with Gremlin. I think I'd still recommend that users form their results explicitly and I think that even if we eventually choose to add properties to GraphBinary and allow them to be returned, we'd still want to allow users to do that somehow as opposed to just returning everything. 


----------------------------------------------------------------
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.

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



[GitHub] [tinkerpop] spmallette commented on pull request #1374: Vertex and Edge's properties in graphbinary serialization

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


   GraphBinary was specifically built as a network serialization format whereas kryo/graphson play the double role of network and file serialization. We might have already removed properties from the latter if it didn't break the file serialization role. Note that by default neither of those serialization formats return properties anyway (at least in Gremlin Server as of 3.4.0) as graph elements are converted to references before serialization occurs.  So, even if GraphBinary suddenly started to have property serialization it wouldn't have anything to serialize. Note that Gryo is likely retired as a network serialization format so I don't imagine we'll change anything there ever and GraphSON is likely supplanted by GraphBinary for network serialization so I don't see much need to make changes to either of those for this purpose going forward.
   
   Speaking specifically to that statement, item 5, that you referenced. I was largely pointing out that we were hedging our bets a bit by leaving space for properties in the future, but I'm quite against adding them in isolation without wider thought/discussion to all the other issues in that email. I hope that answers your question.


----------------------------------------------------------------
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.

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