Potential breaking changes in 0.2.0


david.c...@...
 

I add this thread to discuss about breaking change in 0.2.0 with this PR https://github.com/JanusGraph/janusgraph/pull/336.

This PR add a breaking change for Elasticsearch backend because it Elasticsearch split index into N indexes.
To deals with this, we have three options :
  1.  0.2.0 is not compatible with 0.1.x
  2. We create a script to migrate index.
  3. We make the configuration switch.
Between 2 and 3, I prefer 2 because I would like to add a delete method by index to solve https://github.com/JanusGraph/janusgraph/issues/354 and to remove this https://github.com/JanusGraph/janusgraph/blob/master/janusgraph-core/src/main/java/org/janusgraph/graphdb/olap/job/IndexRemoveJob.java#L105

But, for me 0.2.0 is already not compatible with 0.1.x because Geoshape serialization (https://github.com/JanusGraph/janusgraph/blob/master/janusgraph-core/src/main/java/org/janusgraph/core/attribute/Geoshape.java#L542).
In 0.1.x Geoshape serialization was en tab of float, in 0.2.0 JanusGraph use JtsBinaryCodec. So a geoshape serializes in 0.1.x can not be read by 0.2.0. 

What do you think ?

I did not know if there are any other breaking change.

David


Davide Bolcioni <dbol...@...>
 

I would expect an incompatible change to bump the major version, so 1.*

On Wed, Jun 28, 2017 at 9:00 AM, david.clement90 via JanusGraph developer list <janusgr...@...> wrote:
I add this thread to discuss about breaking change in 0.2.0 with this PR https://github.com/JanusGraph/janusgraph/pull/336.

This PR add a breaking change for Elasticsearch backend because it Elasticsearch split index into N indexes.
To deals with this, we have three options :
  1.  0.2.0 is not compatible with 0.1.x
  2. We create a script to migrate index.
  3. We make the configuration switch.

But, for me 0.2.0 is already not compatible with 0.1.x because Geoshape serialization (https://github.com/JanusGraph/janusgraph/blob/master/janusgraph-core/src/main/java/org/janusgraph/core/attribute/Geoshape.java#L542).
In 0.1.x Geoshape serialization was en tab of float, in 0.2.0 JanusGraph use JtsBinaryCodec. So a geoshape serializes in 0.1.x can not be read by 0.2.0. 

What do you think ?

I did not know if there are any other breaking change.

David

--
You received this message because you are subscribed to the Google Groups "JanusGraph developer list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgraph-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
There is no place like /home.


sjudeng <sju...@...>
 

Although it's not as clean I think it's worth pursing the configuration option. You could call it something obtrusive like "index.elasicsearch.use-deprecated-multitype-index". If possible I think we'd want it to default to false so all new indices are split, but be treated as true if not present as in the case for legacy indices. As far as implementation complexity, would it maybe be as simple as adding logic to your ElasticSearchIndex.getIndexStoreName() method? Regarding your desired index deletion work, could you move forward if support is limited to single-type indices (e.g. so there'd be an additional conditional at https://github.com/JanusGraph/janusgraph/blob/bfa3c970555932d4983155168322f72297aaee85/janusgraph-core/src/main/java/org/janusgraph/graphdb/olap/job/IndexRemoveJob.java#L104)?

I'd still like to hear from others as well but this is what I'm thinking.


On Wednesday, June 28, 2017 at 11:47:49 AM UTC-5, Davide Bolcioni wrote:
I would expect an incompatible change to bump the major version, so 1.*

On Wed, Jun 28, 2017 at 9:00 AM, david.clement90 via JanusGraph developer list <janusgr...@googlegroups.com> wrote:
I add this thread to discuss about breaking change in 0.2.0 with this PR https://github.com/JanusGraph/janusgraph/pull/336.

This PR add a breaking change for Elasticsearch backend because it Elasticsearch split index into N indexes.
To deals with this, we have three options :
  1.  0.2.0 is not compatible with 0.1.x
  2. We create a script to migrate index.
  3. We make the configuration switch.

But, for me 0.2.0 is already not compatible with 0.1.x because Geoshape serialization (https://github.com/JanusGraph/janusgraph/blob/master/janusgraph-core/src/main/java/org/janusgraph/core/attribute/Geoshape.java#L542).
In 0.1.x Geoshape serialization was en tab of float, in 0.2.0 JanusGraph use JtsBinaryCodec. So a geoshape serializes in 0.1.x can not be read by 0.2.0. 

What do you think ?

I did not know if there are any other breaking change.

David

--
You received this message because you are subscribed to the Google Groups "JanusGraph developer list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgraph-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
There is no place like /home.


david.c...@...
 

Yes I will add a logic to the getIndexStoreName() and also when JanusGraph query elasticsearch.

For the index delation work, if the index is not split, I can do it but it can take a long long time : it's depending of the number of document.

Le vendredi 30 juin 2017 04:31:27 UTC+2, sjudeng a écrit :
Although it's not as clean I think it's worth pursing the configuration option. You could call it something obtrusive like "index.elasicsearch.use-deprecated-multitype-index". If possible I think we'd want it to default to false so all new indices are split, but be treated as true if not present as in the case for legacy indices. As far as implementation complexity, would it maybe be as simple as adding logic to your ElasticSearchIndex.getIndexStoreName() method? Regarding your desired index deletion work, could you move forward if support is limited to single-type indices (e.g. so there'd be an additional conditional at https://github.com/JanusGraph/janusgraph/blob/bfa3c970555932d4983155168322f72297aaee85/janusgraph-core/src/main/java/org/janusgraph/graphdb/olap/job/IndexRemoveJob.java#L104)?

I'd still like to hear from others as well but this is what I'm thinking.

On Wednesday, June 28, 2017 at 11:47:49 AM UTC-5, Davide Bolcioni wrote:
I would expect an incompatible change to bump the major version, so 1.*

On Wed, Jun 28, 2017 at 9:00 AM, david.clement90 via JanusGraph developer list <jan...@...> wrote:
I add this thread to discuss about breaking change in 0.2.0 with this PR https://github.com/JanusGraph/janusgraph/pull/336.

This PR add a breaking change for Elasticsearch backend because it Elasticsearch split index into N indexes.
To deals with this, we have three options :
  1.  0.2.0 is not compatible with 0.1.x
  2. We create a script to migrate index.
  3. We make the configuration switch.

But, for me 0.2.0 is already not compatible with 0.1.x because Geoshape serialization (https://github.com/JanusGraph/janusgraph/blob/master/janusgraph-core/src/main/java/org/janusgraph/core/attribute/Geoshape.java#L542).
In 0.1.x Geoshape serialization was en tab of float, in 0.2.0 JanusGraph use JtsBinaryCodec. So a geoshape serializes in 0.1.x can not be read by 0.2.0. 

What do you think ?

I did not know if there are any other breaking change.

David

--
You received this message because you are subscribed to the Google Groups "JanusGraph developer list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgraph-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
There is no place like /home.