feat(store): integrate `store-grpc`&`store-common`&`store-client` #2476

Pengzna · 2024-03-12T06:48:40Z

subtask of #2265, this PR mainly introduce the store-client module

During the code review, I found the following issues:

HgStoreClient can hold multiple HgStoreSession to maintain multiple connections, but now it only holds one.
MultiNodeSessionFactory doesn't consider the case of multiple instances.
HgSessionManager now is constructed by SPI, but it seems to have only one implementation class and thus does not require SPI?
NodeTxSessionProxy.clean()only uses the first node's cleaning result to determine whether the overall failure occurs. If subsequent nodes fail, we will not be able to detect it.
Duplicate code: like getHeader() method

For the store-client submodule, its structure can be seen in detailed docs.

Note: code style including comments will be handled uniformly in future related PRs

codecov · 2024-03-12T06:50:56Z

Codecov Report

Attention: Patch coverage is 0% with 1664 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (pd-store-dev@3dd06f2). Click here to learn what that means.

Files	Patch %	Lines
...che/hugegraph/store/client/NodeTxSessionProxy.java	0.00%	403 Missing ⚠️
.../apache/hugegraph/store/client/NodeTxExecutor.java	0.00%	169 Missing ⚠️
...n/java/org/apache/hugegraph/store/HgScanQuery.java	0.00%	103 Missing ⚠️
...graph/store/client/HgStoreNodePartitionerImpl.java	0.00%	95 Missing ⚠️
...che/hugegraph/store/client/HgStoreNodeManager.java	0.00%	85 Missing ⚠️
...org/apache/hugegraph/store/client/util/Base58.java	0.00%	74 Missing ⚠️
...gegraph/store/client/util/HgStoreClientConfig.java	0.00%	73 Missing ⚠️
...hugegraph/store/client/util/HgStoreClientUtil.java	0.00%	71 Missing ⚠️
...che/hugegraph/store/client/util/HgBufferProxy.java	0.00%	61 Missing ⚠️
...g/apache/hugegraph/store/client/util/HgAssert.java	0.00%	53 Missing ⚠️
... and 32 more

Additional details and impacted files

@@               Coverage Diff               @@
##             pd-store-dev    #2476   +/-   ##
===============================================
  Coverage                ?   61.07%           
  Complexity              ?      827           
===============================================
  Files                   ?      579           
  Lines                   ?    46206           
  Branches                ?     6280           
===============================================
  Hits                    ?    28220           
  Misses                  ?    15169           
  Partials                ?     2817

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/pd-store.yml

VGalaxies

LGTM~

javeme · 2024-03-23T14:16:14Z

hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/HgTkvEntry.java

+
+package org.apache.hugegraph.store;
+
+public interface HgTkvEntry {


this name is a bit hard to understand, to be improved

Yes, we will fix it in the next code style related PR

hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/HgTokvEntry.java

javeme · 2024-03-23T14:17:36Z

hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/HgPrivate.java

+/**
+ * created on 2021/10/26
+ */
+public class HgPrivate {


this name is a bit hard to understand, to be improved

ditto, and this class seems to be useless, we will consider to remove it in futhur PR

...h-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/HgNodePartition.java

hugegraph-store/hg-store-grpc/src/main/proto/store_session.proto

hugegraph-store/hg-store-grpc/src/main/proto/store_common.proto

javeme · 2024-03-23T16:25:42Z

hugegraph-store/hg-store-grpc/src/main/proto/store_session.proto

+  rpc Table(TableReq) returns (FeedbackRes){};
+  rpc Graph(GraphReq) returns (FeedbackRes){};


Table/Graph looks a little strange, where are these two interfaces used , and what content do they return?

They are mainly used in org.apache.hugegraph.store.node.grpc.HgStoreNodeService (the related migration has not been completed yet, and store-node will be migrated after the migration of this pr, store-core and store-rocksdb).

These two interfaces mainly define the CRUD operations related to table and graph. They will return true or false depending on whether these operations have failed or not.

javeme

TODO: fix comments later

VGalaxies · 2024-04-02T03:10:38Z

merge directly into the master branch after #2498...

The base branch was changed.

codecov-commenter · 2024-04-03T11:29:01Z

Codecov Report

Attention: Patch coverage is 0% with 1664 lines in your changes are missing coverage. Please review.

Project coverage is 58.83%. Comparing base (3a1618f) to head (6be946b).

Files	Patch %	Lines
...che/hugegraph/store/client/NodeTxSessionProxy.java	0.00%	403 Missing ⚠️
.../apache/hugegraph/store/client/NodeTxExecutor.java	0.00%	169 Missing ⚠️
...n/java/org/apache/hugegraph/store/HgScanQuery.java	0.00%	103 Missing ⚠️
...graph/store/client/HgStoreNodePartitionerImpl.java	0.00%	95 Missing ⚠️
...che/hugegraph/store/client/HgStoreNodeManager.java	0.00%	85 Missing ⚠️
...org/apache/hugegraph/store/client/util/Base58.java	0.00%	74 Missing ⚠️
...gegraph/store/client/util/HgStoreClientConfig.java	0.00%	73 Missing ⚠️
...hugegraph/store/client/util/HgStoreClientUtil.java	0.00%	71 Missing ⚠️
...che/hugegraph/store/client/util/HgBufferProxy.java	0.00%	61 Missing ⚠️
...g/apache/hugegraph/store/client/util/HgAssert.java	0.00%	53 Missing ⚠️
... and 32 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2476      +/-   ##
============================================
- Coverage     65.64%   58.83%   -6.81%     
  Complexity      827      827              
============================================
  Files           518      579      +61     
  Lines         42987    46232    +3245     
  Branches       5976     6275     +299     
============================================
- Hits          28218    27202    -1016     
- Misses        11957    16275    +4318     
+ Partials       2812     2755      -57

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fix: maven rat plugin exist partial failure

imbajin

I'll enhance some TODOs later (merge it first)

imbajin changed the title ~~chore(pd-store-dev): integrate store-grpc, store-common, store-client into hugegraph~~ feat(store-dev): integrate store-grpc, store-common, store-client into hugegraph Mar 12, 2024

VGalaxies mentioned this pull request Mar 12, 2024

[Summary] integrate new modules PD and Store components into hugegraph (Breaking Change) #2265

Closed

26 tasks

VGalaxies self-assigned this Mar 12, 2024

VGalaxies added the store Store module label Mar 12, 2024

VGalaxies self-requested a review March 12, 2024 06:51

JackyYangPassion reviewed Mar 12, 2024

View reviewed changes

.github/workflows/pd-store.yml Outdated Show resolved Hide resolved

Pengzna marked this pull request as ready for review March 16, 2024 11:40

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. ci-cd Build or deploy feature New feature labels Mar 16, 2024

Pengzna mentioned this pull request Mar 16, 2024

[Summary] integrate new module Store into hugegraph #2482

Closed

7 tasks

VGalaxies previously approved these changes Mar 17, 2024

View reviewed changes

VGalaxies requested a review from JackyYangPassion March 17, 2024 03:21

dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 17, 2024

javeme reviewed Mar 23, 2024

View reviewed changes

imbajin mentioned this pull request Mar 25, 2024

feat(pd): integrate pd-core into hugegraph #2478

Merged

Pengzna requested a review from javeme March 25, 2024 04:02

javeme previously approved these changes Mar 30, 2024

View reviewed changes

JackyYangPassion previously approved these changes Apr 2, 2024

View reviewed changes

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Apr 3, 2024

Pengzna changed the base branch from pd-store-dev to master April 3, 2024 10:57

Pengzna requested a review from VGalaxies April 3, 2024 10:57

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Apr 3, 2024

Pengzna force-pushed the pd-store-dev branch from 74ae4af to e9d5427 Compare April 5, 2024 04:00

Pengzna added 5 commits April 5, 2024 12:15

refact: prepare for integrating store modules

dd8a3d3

feat(store): integrate store-common submodule

2a39813

feat(store): integrate store-grpc submodule

0f820a7

feat(store): integrate store-client submodule

17a3735

feat(store): integrate store-test submodule

f59cc13

Pengzna force-pushed the pd-store-dev branch from e9d5427 to f59cc13 Compare April 5, 2024 04:16

VGalaxies previously approved these changes Apr 5, 2024

View reviewed changes

imbajin dismissed VGalaxies’s stale review via 03904ae April 5, 2024 07:58

fix: maven rat plugin exist partial failure

6be946b

fix: maven rat plugin exist partial failure

imbajin force-pushed the pd-store-dev branch from fb79bf7 to 6be946b Compare April 5, 2024 09:26

VGalaxies approved these changes Apr 5, 2024

View reviewed changes

VGalaxies changed the title ~~feat(store-dev): integrate store-grpc, store-common, store-client into hugegraph~~ feat(store): integrate store-grpc&store-common&store-client Apr 5, 2024

imbajin approved these changes Apr 5, 2024

View reviewed changes

imbajin merged commit 1228dec into apache:master Apr 5, 2024
14 checks passed

imbajin added this to the 1.5.0 milestone Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(store): integrate `store-grpc`&`store-common`&`store-client` #2476

feat(store): integrate `store-grpc`&`store-common`&`store-client` #2476

Pengzna commented Mar 12, 2024 •

edited by imbajin

Loading

codecov bot commented Mar 12, 2024 •

edited

Loading

VGalaxies left a comment

javeme Mar 23, 2024

Pengzna Mar 25, 2024

javeme Mar 23, 2024

Pengzna Mar 25, 2024

javeme Mar 23, 2024

Pengzna Mar 25, 2024

javeme left a comment

VGalaxies commented Apr 2, 2024

codecov-commenter commented Apr 3, 2024 •

edited

Loading

imbajin left a comment


		package org.apache.hugegraph.store;

		public interface HgTkvEntry {

		rpc Table(TableReq) returns (FeedbackRes){};
		rpc Graph(GraphReq) returns (FeedbackRes){};

feat(store): integrate store-grpc&store-common&store-client #2476

feat(store): integrate store-grpc&store-common&store-client #2476

Conversation

Pengzna commented Mar 12, 2024 • edited by imbajin Loading

codecov bot commented Mar 12, 2024 • edited Loading

Codecov Report

VGalaxies left a comment

Choose a reason for hiding this comment

javeme Mar 23, 2024

Choose a reason for hiding this comment

Pengzna Mar 25, 2024

Choose a reason for hiding this comment

javeme Mar 23, 2024

Choose a reason for hiding this comment

Pengzna Mar 25, 2024

Choose a reason for hiding this comment

javeme Mar 23, 2024

Choose a reason for hiding this comment

Pengzna Mar 25, 2024

Choose a reason for hiding this comment

javeme left a comment

Choose a reason for hiding this comment

VGalaxies commented Apr 2, 2024

codecov-commenter commented Apr 3, 2024 • edited Loading

Codecov Report

imbajin left a comment

Choose a reason for hiding this comment

feat(store): integrate `store-grpc`&`store-common`&`store-client` #2476

feat(store): integrate `store-grpc`&`store-common`&`store-client` #2476

Pengzna commented Mar 12, 2024 •

edited by imbajin

Loading

codecov bot commented Mar 12, 2024 •

edited

Loading

codecov-commenter commented Apr 3, 2024 •

edited

Loading