feat(hstore): Hstore support bulkload #2685
Conversation
dosubot
bot
added
size:XXL
Hstore bulkload
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/PartitionAPI.java
Fixed
Show fixed
Hide fixed
Hstore bulkload
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2685 +/- ##
=============================================
- Coverage 47.68% 34.83% -12.86%
+ Complexity 821 383 -438
=============================================
Files 719 721 +2
Lines 58914 59423 +509
Branches 7595 7663 +68
=============================================
- Hits 28096 20701 -7395
- Misses 28007 36435 +8428
+ Partials 2811 2287 -524 ☔ View full report in Codecov by Sentry. |
Hstore bulkload
Hstore bulkload
Hstore bulkload
There was a problem hiding this comment.
Pull Request Overview
This PR implements the bulkload feature for Hstore support. It introduces new REST endpoints, protobuf message definitions, and task scheduling logic to support bulkload operations.
- Added a new bulkload method in the REST service and corresponding API endpoints.
- Updated protobuf definitions and task metadata to include bulkload task information.
- Extended internal services to handle bulkload task creation, reporting, and HDFS file operations.
Reviewed Changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java | Adds a bulkload method invoking store node bulkload operations. |
| hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/TaskAPI.java | Introduces REST endpoints for bulkloading and leader redirection logic. |
| hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/PartitionAPI.java | Adds an endpoint to retrieve partition and graph ID mapping. |
| hg-pd-service/src/main/java/org/apache/hugegraph/pd/model/BulkloadRestRequest.java | Defines the bulkload request payload. |
| hg-pd-grpc/src/main/proto/pd_pulse.proto, metapb.proto, metaTask.proto | Updates protobuf definitions to include bulkload task and info. |
| hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/TaskInfoMeta.java, PartitionMeta.java, MetadataKeyHelper.java | Introduces bulkload task methods and key helpers for metadata storage. |
| hg-pd-core/src/main/java/org/apache/hugegraph/pd/TaskScheduleService.java, StoreNodeService.java, PartitionService.java | Implements bulkload task scheduling, processing, and status reporting. |
| hg-pd-common/src/main/java/org/apache/hugegraph/pd/common/HdfsUtils.java | Adds utility methods for parsing HDFS file paths and downloading files. |
| hg-pd-client/src/main/java/org/apache/hugegraph/pd/client/PDClient.java | Provides a new client method for querying leader partitions. |
Files not reviewed (1)
- hugegraph-pd/hg-pd-common/pom.xml: Language not supported
| while (true) { | ||
|
|
There was a problem hiding this comment.
Consider adding a timeout or exit condition to this indefinite loop to prevent a potential hang if the bulkload task status never reaches a terminal state.
| while (true) { | |
| int maxRetries = 60; // Maximum number of retries (10 minutes) | |
| int retries = 0; | |
| while (retries < maxRetries) { |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| if (statusMap.get(partition.getId()).state != null && | ||
| statusMap.get(partition.getId()).state != MetaTask.TaskState.Task_Ready) { | ||
| var newTask = | ||
| pdMetaTask.toBuilder().setState(statusMap.get(partition.getId()).state).build(); | ||
|
|
There was a problem hiding this comment.
[nitpick] Store the result of 'statusMap.get(partition.getId())' in a local variable to avoid repeated lookups and to improve code readability.
| if (statusMap.get(partition.getId()).state != null && | |
| statusMap.get(partition.getId()).state != MetaTask.TaskState.Task_Ready) { | |
| var newTask = | |
| pdMetaTask.toBuilder().setState(statusMap.get(partition.getId()).state).build(); | |
| var partitionStatus = statusMap.get(partition.getId()); | |
| if (partitionStatus.state != null && | |
| partitionStatus.state != MetaTask.TaskState.Task_Ready) { | |
| var newTask = | |
| pdMetaTask.toBuilder().setState(partitionStatus.state).build(); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| return handleBulkload(request); | ||
| } else { | ||
| String leaderAddress = RaftEngine.getInstance().getLeader().getIp(); | ||
| String url = "http://" + leaderAddress+":8620" + "/v1/task/bulkload"; |
There was a problem hiding this comment.
[nitpick] Consider extracting the hardcoded port value into a constant or configuration property to simplify future updates and improve maintainability.
| String url = "http://" + leaderAddress+":8620" + "/v1/task/bulkload"; | |
| String url = "http://" + leaderAddress + ":" + LEADER_PORT + "/v1/task/bulkload"; |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
refer #2669
as mentioned in the discussions, this is the code for implementing the bulkload feature.
@JackyYangPassion @imbajin @VGalaxies please review ,please let me know if you have any questions. Thank you