[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1215209736
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> Yeah, when a new client id is generated, the client would immediately sent
a heart beat to the table, while other clients see the heartbeat, they skip the
occupied hearbeated ids, can you help to check whether that works as expected?
OK
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1214344879
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> > all writer will shared the ckp_meta
>
> How could that happen then providing the last client already sent the
heartbeat?
Sorry,i am not very familiar with heartbeat logic,if my understanding is
wrong,i can close this pr.
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1213854825
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> It should be a bug, the client still send heartbeat anyway for the
INIT_CLIEN _ID:
>
>
https://github.com/apache/hudi/blob/00d50e91abe24aba31daa2fe2806de5414f03c77/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/util/ClientIds.java#L179
Maybe,bug if the INIT_CLIENT_ID is empty,all writer will shared the
ckp_meta,the risk of concurrent modification will occupy.
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1212716258
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> > so the ckp_meta is shared by all writers.
>
> The writer should start a new client id if it saw the clienid with empty
string is already occupied.
Actually,the writer doesn't generate a new client,because client id
generated for the first time not empty.
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1212641835
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> Can you explain again why we need this fix, when a Flink job was
submitted, it is assigned a client id automically, this id is hold by this job
with a hearbeat, when another job was submitted, it saw the heartbeat and would
create a new client id, is this what you saw in you case?
when a Flink job started,it is assigned a client id ,but the client id is a
empty string,so the ckp_meta is shared by all writers.
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1211347431
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
> The `INIT_CLIENT_ID` is not null, it is an empty string, did you set up
the `hoodie.write.concurrency.mode` as `OPTIMISTIC_CONCURRENCY_CONTROL` ?
yes, i set up the `hoodie.write.concurrency.mode` as
`OPTIMISTIC_CONCURRENCY_CONTROL`
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1209016260
##
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/configuration/TestOptionsInference.java:
##
@@ -69,6 +70,12 @@ void testSetupClientId() throws Exception {
}
}
+ @Test
+ void testAutoGenerateClient() {
+ Configuration conf = getConf();
+ OptionsInference.setupClientId(conf);
+ assertNotNull(conf.getString(FlinkOptions.WRITE_CLIENT_ID), "auto
generate client failed!");
+ }
Review Comment:
This pr just fix the INIT_CLIENT_ID,in the ooc scene,if we don't set the
config`write.client.id`,it will auto generate the CLIENT,and the INIT_CLIENT_ID
doesn't null.
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] c-f-cooper commented on a diff in pull request #8830: [MINOR] auto generate init client id
c-f-cooper commented on code in PR #8830:
URL: https://github.com/apache/hudi/pull/8830#discussion_r1208269112
##
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/util/ClientIds.java:
##
@@ -69,7 +70,7 @@ public class ClientIds implements AutoCloseable, Serializable
{
private static final String HEARTBEAT_FOLDER_NAME = ".ids";
private static final String HEARTBEAT_FILE_NAME_PREFIX = "_";
- public static final String INIT_CLIENT_ID = "";
+ public static final String INIT_CLIENT_ID =
String.valueOf(RandomUtils.nextInt(0,100));
public static final long DEFAULT_HEARTBEAT_INTERVAL_IN_MS = 60 * 1000; //
default 1 minute
Review Comment:
> Can we use Java util `Random` instead commons lang3, the apache commons is
likely to conflict.
OK
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
