You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by "e-mhui (via GitHub)" <gi...@apache.org> on 2023/03/27 13:55:36 UTC

[GitHub] [inlong] e-mhui opened a new pull request, #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

e-mhui opened a new pull request, #7709:
URL: https://github.com/apache/inlong/pull/7709

   ### Prepare a Pull Request
   
   [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle
   
   - Fixes #7708
   
   ### Motivation
   
   Fix parse source failed when adding a table during all database migration in Oracle.
   When adding a new table, the `schemaChangeRecordValue` method reads three values from the `source`: `BINLOG_FILENAME_OFFSET_KEY`, `BINLOG_POSITION_OFFSET_KEY` and `SERVER_ID_KEY`. However, these fields only exist in MySQL, so we need to check the type of the source.
   
   ### Modifications
   
   Check if the source is of type MySQL, and if it is, obtain the BINLOG-related information.
   
   ```java
               if (MYSQL.equals(connector)) {
                   String fileName = sourceInfo.getString(BINLOG_FILENAME_OFFSET_KEY);
                   Long pos = sourceInfo.getInt64(BINLOG_POSITION_OFFSET_KEY);
                   Long serverId = sourceInfo.getInt64(SERVER_ID_KEY);
                   source.put(SERVER_ID_KEY, serverId);
                   source.put(BINLOG_FILENAME_OFFSET_KEY, fileName);
                   source.put(BINLOG_POSITION_OFFSET_KEY, pos);
               
   ```


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] e-mhui commented on pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "e-mhui (via GitHub)" <gi...@apache.org>.
e-mhui commented on PR #7709:
URL: https://github.com/apache/inlong/pull/7709#issuecomment-1486362097

   > > > @e-mhui `JdbcSourceEventDispatcher` package path of oracle-cdc is same with cdc-base . I think this is not good. It will cause class conflict when class load mutil cdc jar.
   > > 
   > > 
   > > 
   > > 1. Inlong has multiple versions of dependencies for debezium. cdc-base depends on version 1.5.4-final, but Oracle CDC depends on version 1.6.4-final. The method   `HistorizedSchema.storeOnlyCapturedTables()`   is only available in version 1.6.4-final. If we change it to use   `historizedSchema.storeOnlyMonitoredTables()`   in version 1.5.4-final, it may cause conflicts . Therefore, the method is overwritten in Oracle. After all CDCs are upgraded to version 2.3, we can use   `JdbcSourceEventDispatcher`   in cdc-base uniformly.
   > > 
   > > ```java
   > >     @Override
   > >     public void dispatchSchemaChangeEvent(
   > >             TableId dataCollectionId, SchemaChangeEventEmitter schemaChangeEventEmitter)
   > >             throws InterruptedException {
   > >         if (dataCollectionId != null && !filter.isIncluded(dataCollectionId)) {
   > >             // TODO `HistorizedSchema.storeOnlyCapturedTables` first appeared in io.debezium.core: 1.6.4-final,
   > >             // which conflicts with io.debezium.core: 15.4-final of cdc-base,
   > >             // so we implement this method separately in the sub-module.
   > >             // if (historizedSchema == null || historizedSchema.storeOnlyCapturedTables()) {
   > >             if (historizedSchema == null || historizedSchema.storeOnlyMonitoredTables()) {
   > >                 LOG.trace("Filtering schema change event for {}", dataCollectionId);
   > >                 return;
   > >             }
   > >         }
   > >         schemaChangeEventEmitter.emitSchemaChangeEvent(new SchemaChangeEventReceiver());
   > >     }
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > 
   > > 2. If there are conflicts, I suggest resolving them using the shade.
   > 
   > @e-mhui Because package path is same. So shade won't be feasible. why not to modify directly package path
   
   done.


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] e-mhui commented on pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "e-mhui (via GitHub)" <gi...@apache.org>.
e-mhui commented on PR #7709:
URL: https://github.com/apache/inlong/pull/7709#issuecomment-1486222731

   > @e-mhui `JdbcSourceEventDispatcher` package path of oracle-cdc is same with cdc-base . I think this is not good. It will cause class conflict when class load mutil cdc jar.
   
   1. Inlong has multiple versions of dependencies for debezium. cdc-base depends on version 1.5.4-final, but Oracle CDC depends on version 1.6.4-final. The method   `HistorizedSchema.storeOnlyCapturedTables()`   is only available in version 1.6.4-final. If we change it to use   `historizedSchema.storeOnlyMonitoredTables()`   in version 1.5.4-final, it may cause conflicts . Therefore, the method is overwritten in Oracle. After all CDCs are upgraded to version 2.3, we can use   `JdbcSourceEventDispatcher`   in cdc-base uniformly.
   
   ```java
       @Override
       public void dispatchSchemaChangeEvent(
               TableId dataCollectionId, SchemaChangeEventEmitter schemaChangeEventEmitter)
               throws InterruptedException {
           if (dataCollectionId != null && !filter.isIncluded(dataCollectionId)) {
               // TODO `HistorizedSchema.storeOnlyCapturedTables` first appeared in io.debezium.core: 1.6.4-final,
               // which conflicts with io.debezium.core: 15.4-final of cdc-base,
               // so we implement this method separately in the sub-module.
               // if (historizedSchema == null || historizedSchema.storeOnlyCapturedTables()) {
               if (historizedSchema == null || historizedSchema.storeOnlyMonitoredTables()) {
                   LOG.trace("Filtering schema change event for {}", dataCollectionId);
                   return;
               }
           }
           schemaChangeEventEmitter.emitSchemaChangeEvent(new SchemaChangeEventReceiver());
       }
   ```
   
   2. If there are conflicts, I suggest resolving them using the shade.
   
   


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] EMsnap commented on a diff in pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "EMsnap (via GitHub)" <gi...@apache.org>.
EMsnap commented on code in PR #7709:
URL: https://github.com/apache/inlong/pull/7709#discussion_r1150073682


##########
inlong-sort/sort-connectors/cdc-base/src/main/java/org/apache/inlong/sort/cdc/base/util/RecordUtils.java:
##########
@@ -130,4 +135,24 @@ public static LogicalType convertLogicType(Column column, Struct struct) {
         return null;
     }
 
+    /**
+     * Whether the source Record is a schema change event.
+     * @param sourceRecord
+     * @retur ture or false

Review Comment:
   typo 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] gong commented on pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "gong (via GitHub)" <gi...@apache.org>.
gong commented on PR #7709:
URL: https://github.com/apache/inlong/pull/7709#issuecomment-1486280714

   > > @e-mhui `JdbcSourceEventDispatcher` package path of oracle-cdc is same with cdc-base . I think this is not good. It will cause class conflict when class load mutil cdc jar.
   > 
   > 1. Inlong has multiple versions of dependencies for debezium. cdc-base depends on version 1.5.4-final, but Oracle CDC depends on version 1.6.4-final. The method   `HistorizedSchema.storeOnlyCapturedTables()`   is only available in version 1.6.4-final. If we change it to use   `historizedSchema.storeOnlyMonitoredTables()`   in version 1.5.4-final, it may cause conflicts . Therefore, the method is overwritten in Oracle. After all CDCs are upgraded to version 2.3, we can use   `JdbcSourceEventDispatcher`   in cdc-base uniformly.
   > 
   > ```java
   >     @Override
   >     public void dispatchSchemaChangeEvent(
   >             TableId dataCollectionId, SchemaChangeEventEmitter schemaChangeEventEmitter)
   >             throws InterruptedException {
   >         if (dataCollectionId != null && !filter.isIncluded(dataCollectionId)) {
   >             // TODO `HistorizedSchema.storeOnlyCapturedTables` first appeared in io.debezium.core: 1.6.4-final,
   >             // which conflicts with io.debezium.core: 15.4-final of cdc-base,
   >             // so we implement this method separately in the sub-module.
   >             // if (historizedSchema == null || historizedSchema.storeOnlyCapturedTables()) {
   >             if (historizedSchema == null || historizedSchema.storeOnlyMonitoredTables()) {
   >                 LOG.trace("Filtering schema change event for {}", dataCollectionId);
   >                 return;
   >             }
   >         }
   >         schemaChangeEventEmitter.emitSchemaChangeEvent(new SchemaChangeEventReceiver());
   >     }
   > ```
   > 
   > 2. If there are conflicts, I suggest resolving them using the shade.
   
   @e-mhui Because package path is same. So shade won't be feasible. 


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] gong commented on pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "gong (via GitHub)" <gi...@apache.org>.
gong commented on PR #7709:
URL: https://github.com/apache/inlong/pull/7709#issuecomment-1486295779

   <img width="1548" alt="image" src="https://user-images.githubusercontent.com/9883232/228149293-24782a1b-ad90-47fb-8492-c9623825127d.png">
   @e-mhui I find a old bug, class path is error
   


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] dockerzhang merged pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang merged PR #7709:
URL: https://github.com/apache/inlong/pull/7709


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] e-mhui commented on a diff in pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "e-mhui (via GitHub)" <gi...@apache.org>.
e-mhui commented on code in PR #7709:
URL: https://github.com/apache/inlong/pull/7709#discussion_r1150077090


##########
inlong-sort/sort-connectors/cdc-base/src/main/java/org/apache/inlong/sort/cdc/base/util/RecordUtils.java:
##########
@@ -130,4 +135,24 @@ public static LogicalType convertLogicType(Column column, Struct struct) {
         return null;
     }
 
+    /**
+     * Whether the source Record is a schema change event.
+     * @param sourceRecord
+     * @retur ture or false

Review Comment:
   done.



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [inlong] gong commented on pull request #7709: [INLONG-7708][Sort] Fix parse source failed when adding a table during all database migration in Oracle

Posted by "gong (via GitHub)" <gi...@apache.org>.
gong commented on PR #7709:
URL: https://github.com/apache/inlong/pull/7709#issuecomment-1486189876

   @e-mhui `JdbcSourceEventDispatcher` package path is same with cdc-base of oracle-cdc. I think this is not good. It will cause class confict when class load mutil cdc jar.


-- 
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: commits-unsubscribe@inlong.apache.org

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