Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce the storage of vertex/edge id #661

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Reduce the storage of vertex/edge id #661

merged 4 commits into from
Sep 12, 2019

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Aug 24, 2019

Change-Id: I374deb904ae8ca20b5768ebc7976f831ec558e3e

@codecov
Copy link

codecov bot commented Sep 3, 2019 •
edited

Codecov Report

Merging #661 into master will decrease coverage by 0.6%.
The diff coverage is 80.2%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #661      +/-   ##
============================================
- Coverage     72.96%   72.35%   -0.61%     
- Complexity     3944     4400     +456     
============================================
  Files           219      220       +1     
  Lines         17867    19474    +1607     
  Branches       2569     3021     +452     
============================================
+ Hits          13036    14091    +1055     
- Misses         3489     3984     +495     
- Partials       1342     1399      +57
Impacted Files Coverage Δ Complexity Δ
...u/hugegraph/schema/builder/VertexLabelBuilder.java 87.06% <ø> (ø) 65 <0> (ø) ⬇️
...n/java/com/baidu/hugegraph/schema/VertexLabel.java 100% <ø> (ø) 8 <0> (ø) ⬇️
...m/baidu/hugegraph/exception/NotFoundException.java 33.33% <0%> (-16.67%) 1 <0> (ø)
...gegraph/backend/store/scylladb/ScyllaDBTables.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...graph/backend/store/cassandra/CassandraTables.java 85.97% <100%> (ø) 1 <0> (ø) ⬇️
...idu/hugegraph/backend/store/mysql/MysqlTables.java 88.63% <100%> (ø) 1 <0> (ø) ⬇️
...va/com/baidu/hugegraph/backend/id/IdGenerator.java 79.2% <100%> (+0.2%) 15 <1> (+1) ⬆️
...backend/store/postgresql/PostgresqlSerializer.java 87.5% <100%> (ø) 6 <0> (ø) ⬇️
...ava/com/baidu/hugegraph/structure/HugeElement.java 66.9% <66.66%> (+0.47%) 46 <0> (+1) ⬆️
...hugegraph/backend/serializer/BinarySerializer.java 82.72% <71.42%> (+1.42%) 177 <0> (+72) ⬆️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64f9409...bcbbca9. Read the comment docs.

@javeme
Reduce the storage of vertex/edge id
935e2ef 
Change-Id: I374deb904ae8ca20b5768ebc7976f831ec558e3e
@javeme
fix missing writeStoredString() call
9224f6e 
Change-Id: I5a2647820a8931644526ae3f6b828556211eaf02
@javeme javeme force-pushed the optimize-storage-vertex-edge-id branch from fd66c40 to 9224f6e Compare September 11, 2019 06:14
@javeme
fix unit test
2a6459e 
Change-Id: I65f061c47a1de58197abbb642e0c4524c08481e6
zhoney
zhoney requested changes Sep 12, 2019
View reviewed changes
hugegraph-core/src/main/java/com/baidu/hugegraph/schema/VertexLabel.java
@@ -85,7 +85,7 @@ public void primaryKeys(Id... ids) {

Builder useCustomizeNumberId();

Builder useCustomizeUUid();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think UUID is ok

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/id/IdUtil.java Outdated
private static final String NUMBER_PREFIX = "L";
private static final String STRING_PREFIX = "S";
public static String writeStoredString(Id id) {
String string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to idContent or idString

Linary
Linary requested changes Sep 12, 2019
View reviewed changes
hugegraph-core/src/main/java/com/baidu/hugegraph/backend/id/EdgeId.java Outdated
@@ -132,14 +133,12 @@ public String asString() {
this.cache = SplicingIdGenerator.concat(
IdUtil.writeString(this.ownerVertexId),
this.direction.type().string(),
this.edgeLabelId.asString(),
this.sortValues,
IdUtil.writeLong(this.edgeLabelId), this.sortValues,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let this.sortValues in an single line is more clear

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/id/EdgeId.java Outdated
IdUtil.writeString(this.otherVertexId));
} else {
this.cache = SplicingIdGenerator.concat(
IdUtil.writeString(this.sourceVertexId()),
this.edgeLabelId.asString(),
this.sortValues,
IdUtil.writeLong(this.edgeLabelId), this.sortValues,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/id/EdgeId.java Outdated
EdgeId eid = (EdgeId) id;
return SplicingIdGenerator.concat(
IdUtil.writeStoredString(eid.sourceVertexId()),
IdGenerator.asStoredString(eid.edgeLabelId()), eid.sortValues(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/id/Id.java
EDGE;

public char prefix() {
return this.name().charAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle UNKNOWN and UUID

@javeme
tiny improve
bcbbca9 
Change-Id: I9962b0b1c45676e96b06ea9b85fcb7073c05c6bf
@zhoney zhoney merged commit 1208522 into master Sep 12, 2019
@zhoney zhoney deleted the optimize-storage-vertex-edge-id branch September 12, 2019 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Linary Linary Linary approved these changes

@zhoney zhoney zhoney approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@javeme @Linary @zhoney