You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/14 19:33:13 UTC

[GitHub] [ignite-extensions] shishkovilja opened a new pull request, #199: IGNITE-18209 Reduce binary metadata synchronization time for CDC thro…

shishkovilja opened a new pull request, #199:
URL: https://github.com/apache/ignite-extensions/pull/199

   …ugh Kafka


-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja commented on a diff in pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja commented on code in PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#discussion_r1070906078


##########
modules/cdc-ext/src/test/java/org/apache/ignite/cdc/kafka/CdcKafkaReplicationTest.java:
##########
@@ -72,8 +72,8 @@ public class CdcKafkaReplicationTest extends AbstractReplicationTest {
 
         KAFKA.createTopic(SRC_DEST_TOPIC, DFLT_PARTS, 1);
         KAFKA.createTopic(DEST_SRC_TOPIC, DFLT_PARTS, 1);
-        KAFKA.createTopic(SRC_DEST_META_TOPIC, 1, 1);
-        KAFKA.createTopic(DEST_SRC_META_TOPIC, 1, 1);
+        KAFKA.createTopic(SRC_DEST_META_TOPIC, DFLT_PARTS, 1);

Review Comment:
   Good point. Changes reverted.



##########
modules/cdc-ext/src/test/java/org/apache/ignite/cdc/kafka/CdcKafkaReplicationTest.java:
##########
@@ -72,8 +72,8 @@ public class CdcKafkaReplicationTest extends AbstractReplicationTest {
 
         KAFKA.createTopic(SRC_DEST_TOPIC, DFLT_PARTS, 1);
         KAFKA.createTopic(DEST_SRC_TOPIC, DFLT_PARTS, 1);
-        KAFKA.createTopic(SRC_DEST_META_TOPIC, 1, 1);
-        KAFKA.createTopic(DEST_SRC_META_TOPIC, 1, 1);
+        KAFKA.createTopic(SRC_DEST_META_TOPIC, DFLT_PARTS, 1);

Review Comment:
   Good point! Changes reverted.



-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja closed pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja closed pull request #199: IGNITE-18209 Add check of a lag in a metadata topic
URL: https://github.com/apache/ignite-extensions/pull/199


-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] nizhikov commented on a diff in pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
nizhikov commented on code in PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#discussion_r1065081210


##########
modules/cdc-ext/src/test/java/org/apache/ignite/cdc/kafka/CdcKafkaReplicationTest.java:
##########
@@ -72,8 +72,8 @@ public class CdcKafkaReplicationTest extends AbstractReplicationTest {
 
         KAFKA.createTopic(SRC_DEST_TOPIC, DFLT_PARTS, 1);
         KAFKA.createTopic(DEST_SRC_TOPIC, DFLT_PARTS, 1);
-        KAFKA.createTopic(SRC_DEST_META_TOPIC, 1, 1);
-        KAFKA.createTopic(DEST_SRC_META_TOPIC, 1, 1);
+        KAFKA.createTopic(SRC_DEST_META_TOPIC, DFLT_PARTS, 1);

Review Comment:
   Please, revert these two lines also.
   As metadata topic must have only 1 partition. 



-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja commented on a diff in pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja commented on code in PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#discussion_r1070906078


##########
modules/cdc-ext/src/test/java/org/apache/ignite/cdc/kafka/CdcKafkaReplicationTest.java:
##########
@@ -72,8 +72,8 @@ public class CdcKafkaReplicationTest extends AbstractReplicationTest {
 
         KAFKA.createTopic(SRC_DEST_TOPIC, DFLT_PARTS, 1);
         KAFKA.createTopic(DEST_SRC_TOPIC, DFLT_PARTS, 1);
-        KAFKA.createTopic(SRC_DEST_META_TOPIC, 1, 1);
-        KAFKA.createTopic(DEST_SRC_META_TOPIC, 1, 1);
+        KAFKA.createTopic(SRC_DEST_META_TOPIC, DFLT_PARTS, 1);

Review Comment:
   Good point! I reverted these 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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja commented on pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja commented on PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#issuecomment-1375998036

   > Hello. Please, minify changes. Remove changes that are not related to the fix. Several points I can see: * long -> Duration refactoring * tests improvements with ACL and other additional setup. Let's move them to other PRs.
   
   Hi! I have reverted changes.
   Tests passed: [RunAllTets](https://ci.ignite.apache.org/buildConfiguration/IgniteExtensions_Tests_RunAllTests/6997362?buildTab=dependencies)


-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] nizhikov commented on a diff in pull request #199: IGNITE-18209 Reduce binary metadata synchronization time for CDC thro…

Posted by GitBox <gi...@apache.org>.
nizhikov commented on code in PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#discussion_r1064745237


##########
modules/cdc-ext/src/main/java/org/apache/ignite/cdc/kafka/KafkaToIgniteMetadataUpdater.java:
##########
@@ -47,8 +52,8 @@ public class KafkaToIgniteMetadataUpdater implements AutoCloseable {
     /** Log. */
     private final IgniteLogger log;
 
-    /** The maximum time to complete Kafka related requests, in milliseconds. */
-    private final long kafkaReqTimeout;
+    /** The maximum duration to complete Kafka related requests. */
+    private final Duration reqTimeoutDuration;

Review Comment:
   Changes unrelated. Please, revert.



-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja commented on pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja commented on PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#issuecomment-1383597293

   Actual test runs:
   
   [Extensions RunAllTets](https://ci.ignite.apache.org/buildConfiguration/IgniteExtensions_Tests_RunAllTests/7007707?buildTab=overview&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true)
   [CDC from above build](https://ci.ignite.apache.org/buildConfiguration/IgniteExtensions_Tests_Cdc/7007680?hideProblemsFromDependencies=false&hideTestsFromDependencies=false)


-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] nizhikov commented on pull request #199: IGNITE-18209 Reduce binary metadata synchronization time for CDC thro…

Posted by GitBox <gi...@apache.org>.
nizhikov commented on PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#issuecomment-1375781903

   Hello. Please, minify changes. Remove changes that are not related to the fix.
   Several points I can see:
       * long -> Duration refactoring
       * tests improvements with ACL and other additional setup.
   Let's move them to other PRs.


-- 
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@ignite.apache.org

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


[GitHub] [ignite-extensions] shishkovilja commented on a diff in pull request #199: IGNITE-18209 Add check of a lag in a metadata topic

Posted by GitBox <gi...@apache.org>.
shishkovilja commented on code in PR #199:
URL: https://github.com/apache/ignite-extensions/pull/199#discussion_r1064890043


##########
modules/cdc-ext/src/main/java/org/apache/ignite/cdc/kafka/KafkaToIgniteMetadataUpdater.java:
##########
@@ -47,8 +52,8 @@ public class KafkaToIgniteMetadataUpdater implements AutoCloseable {
     /** Log. */
     private final IgniteLogger log;
 
-    /** The maximum time to complete Kafka related requests, in milliseconds. */
-    private final long kafkaReqTimeout;
+    /** The maximum duration to complete Kafka related requests. */
+    private final Duration reqTimeoutDuration;

Review Comment:
   I have reverted this. Thanks.



-- 
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@ignite.apache.org

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