-
Couldn't load subscription status.
- Fork 563
feat(hstore): Hstore support bulkload #2685
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.
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
base: master
Are you sure you want to change the base?
Conversation
Hstore bulkload
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) { | ||
|
|
Copilot
AI
Apr 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) { |
| 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(); | ||
|
|
Copilot
AI
Apr 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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(); |
| return handleBulkload(request); | ||
| } else { | ||
| String leaderAddress = RaftEngine.getInstance().getLeader().getIp(); | ||
| String url = "http://" + leaderAddress+":8620" + "/v1/task/bulkload"; |
Copilot
AI
Apr 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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"; |
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