You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by GitBox <gi...@apache.org> on 2021/02/02 11:23:31 UTC

[GitHub] [bahir-flink] hackergin opened a new pull request #109: [BAHIR-260] add kudu table writer config.

hackergin opened a new pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109


   Add some  kudu write config that we can config flush interval, flush buffer size, ignore not found error, ignore-duplicate-error.
   
   The default flush-interval is 1000ms.
   
   default buffer-size is 1000,
   
   default ignore-not-found error is false,
   
   default ignore-duplicate is 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.

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



[GitHub] [bahir-flink] eskabetxe commented on pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#issuecomment-814842388


   @hackergin merged, thanks for you contribution


-- 
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.

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



[GitHub] [bahir-flink] eskabetxe merged pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
eskabetxe merged pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109


   


-- 
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.

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



[GitHub] [bahir-flink] gyfora commented on a change in pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#discussion_r601169641



##########
File path: flink-connector-kudu/src/main/java/org/apache/flink/connectors/kudu/connector/writer/KuduWriterConfig.java
##########
@@ -65,6 +101,11 @@ public String toString() {
     public static class Builder {
         private String masters;
         private FlushMode flushMode = FlushMode.AUTO_FLUSH_BACKGROUND;
+        private long timeout = 30000;
+        private int maxBufferSize = 1000;
+        private int flushInterval = 1000;

Review comment:
       Where did you take this defaults from? How will this affect the out of the box performance?




-- 
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.

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



[GitHub] [bahir-flink] hackergin commented on a change in pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
hackergin commented on a change in pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#discussion_r604675205



##########
File path: flink-connector-kudu/src/main/java/org/apache/flink/connectors/kudu/connector/writer/KuduWriterConfig.java
##########
@@ -65,6 +101,16 @@ public String toString() {
     public static class Builder {
         private String masters;
         private FlushMode flushMode = FlushMode.AUTO_FLUSH_BACKGROUND;
+        // Reference from AsyncKuduClientBuilder defaultOperationTimeoutMs.
+        private long timeout = 30000;

Review comment:
       Sure,  I have update the code,  It seems that session-related parameters cannot be directly referenced.




-- 
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.

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



[GitHub] [bahir-flink] eskabetxe commented on a change in pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
eskabetxe commented on a change in pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#discussion_r604290992



##########
File path: flink-connector-kudu/src/main/java/org/apache/flink/connectors/kudu/connector/writer/KuduWriterConfig.java
##########
@@ -65,6 +101,16 @@ public String toString() {
     public static class Builder {
         private String masters;
         private FlushMode flushMode = FlushMode.AUTO_FLUSH_BACKGROUND;
+        // Reference from AsyncKuduClientBuilder defaultOperationTimeoutMs.
+        private long timeout = 30000;

Review comment:
       could we use AsyncKuduClientBuilder.DEFAULT_OPERATION_TIMEOUT_MS directly?
   
   private long timeout = AsyncKuduClientBuilder.DEFAULT_OPERATION_TIMEOUT_MS;




-- 
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.

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



[GitHub] [bahir-flink] gyfora commented on a change in pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#discussion_r603302364



##########
File path: flink-connector-kudu/src/main/java/org/apache/flink/connectors/kudu/connector/writer/KuduWriterConfig.java
##########
@@ -65,6 +101,11 @@ public String toString() {
     public static class Builder {
         private String masters;
         private FlushMode flushMode = FlushMode.AUTO_FLUSH_BACKGROUND;
+        private long timeout = 30000;
+        private int maxBufferSize = 1000;
+        private int flushInterval = 1000;

Review comment:
       sounds good




-- 
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.

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



[GitHub] [bahir-flink] eskabetxe commented on pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#issuecomment-796662332


   @hackergin thanks for your contribution
   @gyfora could you check please


----------------------------------------------------------------
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.

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



[GitHub] [bahir-flink] hackergin commented on a change in pull request #109: [BAHIR-260] add kudu table writer config.

Posted by GitBox <gi...@apache.org>.
hackergin commented on a change in pull request #109:
URL: https://github.com/apache/bahir-flink/pull/109#discussion_r602907335



##########
File path: flink-connector-kudu/src/main/java/org/apache/flink/connectors/kudu/connector/writer/KuduWriterConfig.java
##########
@@ -65,6 +101,11 @@ public String toString() {
     public static class Builder {
         private String masters;
         private FlushMode flushMode = FlushMode.AUTO_FLUSH_BACKGROUND;
+        private long timeout = 30000;
+        private int maxBufferSize = 1000;
+        private int flushInterval = 1000;

Review comment:
       These are from https://github.com/apache/kudu/blob/1eb2a2fbca/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java and  https://github.com/apache/kudu/blob/1eb2a2fbca/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java.  So, It will not affect the out of box performance.  I can add some  Annotation for them.




-- 
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.

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