You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/10/27 09:13:51 UTC

[GitHub] [shardingsphere] azexcy opened a new pull request, #21792: Add create table if not exists at openGauss data source prepare

azexcy opened a new pull request, #21792:
URL: https://github.com/apache/shardingsphere/pull/21792

   
   
   Changes proposed in this pull request:
     - Add create table if not exists at openGauss data source prepare
     - Improve migration native IT
     - Add log
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [ ] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the pull request.
   - [ ] I have passed maven check locally : `mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e`.
   - [ ] I have made corresponding changes to the documentation.
   - [ ] I have added corresponding unit tests for my changes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] sandynz commented on a diff in pull request #21792: Revise pipeline barrier, use job id parameter

Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #21792:
URL: https://github.com/apache/shardingsphere/pull/21792#discussion_r1006759308


##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/PipelineDistributedBarrier.java:
##########
@@ -109,12 +116,13 @@ public void removeParentNode(final String parentPath) {
     /**
      * Await until all children node is ready.
      *
-     * @param parentPath parent path
+     * @param jobId job id
      * @param timeout timeout
      * @param timeUnit time unit
      * @return true if the count reached zero and false if the waiting time elapsed before the count reached zero
      */
-    public boolean await(final String parentPath, final long timeout, final TimeUnit timeUnit) {
+    public boolean await(final String jobId, final long timeout, final TimeUnit timeUnit) {
+        String parentPath = PipelineMetaDataNode.getJobBarrierEnablePath(jobId);

Review Comment:
   The first parameter is changed from `parentPath` to `jobId`, `parentPath` might be `enable` or `disable` path, and now it's hard-coded `enable` path, is it expected?



##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/job/AbstractPipelineJob.java:
##########
@@ -70,7 +69,7 @@ public Collection<Integer> getShardingItems() {
     protected void addTasksRunner(final int shardingItem, final PipelineTasksRunner tasksRunner) {
         tasksRunnerMap.put(shardingItem, tasksRunner);
         PipelineJobProgressPersistService.addJobProgressPersistContext(getJobId(), shardingItem);
-        distributedBarrier.persistEphemeralChildrenNode(PipelineMetaDataNode.getJobBarrierEnablePath(getJobId()), shardingItem);
+        distributedBarrier.persistEphemeralChildrenNode(getJobId(), shardingItem);

Review Comment:
   Looks `persistEphemeralChildrenNode` first parameter is changed to jobId, but it might be `enable` or `disable` path before, is it expected? Or is the impl logic is changed?



##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/PipelineDistributedBarrier.java:
##########
@@ -96,9 +102,10 @@ public void persistEphemeralChildrenNode(final String parentPath, final int shar
     /**
      * Persist ephemeral children node.
      *
-     * @param parentPath parent path
+     * @param jobId job id
      */
-    public void removeParentNode(final String parentPath) {
+    public void unregister(final String jobId) {
+        String parentPath = PipelineMetaDataNode.getJobBarrierDisablePath(jobId);

Review Comment:
   `register` method has hard-coded `enable` path, and this `unregister` method use `disable` path, is it expected?



##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/prepare/datasource/AbstractDataSourcePreparer.java:
##########
@@ -82,8 +82,8 @@ private void executeCreateSchema(final PipelineDataSourceManager dataSourceManag
             try (Statement statement = connection.createStatement()) {
                 statement.execute(sql);
             }
-        } catch (final SQLException ignored) {
-            // TODO should not ignore the exception, if do not catch it, the scaling IT will fail.
+        } catch (final SQLException ex) {
+            log.warn(ex.getMessage());

Review Comment:
   Error message could be more meaningful



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] sandynz merged pull request #21792: Revise pipeline barrier, use job id parameter

Posted by GitBox <gi...@apache.org>.
sandynz merged PR #21792:
URL: https://github.com/apache/shardingsphere/pull/21792


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] azexcy commented on a diff in pull request #21792: Revise pipeline barrier, use job id parameter

Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #21792:
URL: https://github.com/apache/shardingsphere/pull/21792#discussion_r1006843560


##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/job/AbstractPipelineJob.java:
##########
@@ -70,7 +69,7 @@ public Collection<Integer> getShardingItems() {
     protected void addTasksRunner(final int shardingItem, final PipelineTasksRunner tasksRunner) {
         tasksRunnerMap.put(shardingItem, tasksRunner);
         PipelineJobProgressPersistService.addJobProgressPersistContext(getJobId(), shardingItem);
-        distributedBarrier.persistEphemeralChildrenNode(PipelineMetaDataNode.getJobBarrierEnablePath(getJobId()), shardingItem);
+        distributedBarrier.persistEphemeralChildrenNode(getJobId(), shardingItem);

Review Comment:
   Fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org