> > - Regarding names: sure it totally makes sense to follow the kebab case > and Flip has reflected the change. > Regarding the convention, Flink has this widely used configuration > storageDir, which doesn't follow the kebab rule and creates some confusion. > IMHO it would be valuable to add a clear guide.
Ah true, I should have checked the HA-related parameters as well. Initially, I just briefly skimmed over a few ConfigOptions names. @Zhu Zhu Is the alignment of the configuration parameter names also part of the 2.0 efforts that touch the Flink configuration? Is there a guideline we can follow here which is future-proof in terms of parameter naming? - I am considering calling the next method from the Curator framework: > authorization(List<AuthInfo>) [2]. I have added necessary details regarding > Map<String, String> -> List(AuthInfo) conversion, taking into account that > AuthInfo has a constructor with String, byte[] parameters. > The update in the FLIP looks good to me. - Good point. Please let me know if I am missing something, but it seems > that we already can influence ACLProvider for Curator in Flink with > high-availability.zookeeper.client.acl [2] . The way it is done currently > is translation of the predefined constant to some predefined ACL Provider > [3]. I do not see if we can add something to the current FLIP. I suppose > that eventual extension of the supported ACLProvider would be > straightforward and could be done outside of the current Flip as soon > as concrete use-case requirements arise. Thanks for the pointer. My concern is just that, we might have to consider certain formats for the AuthInfo to be aligned with ACLProviders. @Marton: I know that ZooKeeper is probably a bit unrelated to FLIP-211 [1] but since you worked on the Kerberos delegation token provider: Is there something to consider for the ZK Kerberos integration? Maybe, you can help us out. Matthias [1] https://cwiki.apache.org/confluence/display/FLINK/FLIP-211%3A+Kerberos+delegation+token+framework On Mon, Jan 22, 2024 at 6:55 PM Alex Nitavsky <alexnitav...@gmail.com> wrote: > Hello Matthias, > > Thanks a lot for the feedback. > > - Regarding names: sure it totally makes sense to follow the kebab case > and Flip has reflected the change. > Regarding the convention, Flink has this widely used configuration > storageDir, which doesn't follow the kebab rule and creates some confusion. > IMHO it would be valuable to add a clear guide. > > - I am considering calling the next method from the Curator framework: > authorization(List<AuthInfo>) [2]. I have added necessary details regarding > Map<String, String> -> List(AuthInfo) conversion, taking into account that > AuthInfo has a constructor with String, byte[] parameters. > > - Good point. Please let me know if I am missing something, but it seems > that we already can influence ACLProvider for Curator in Flink with > high-availability.zookeeper.client.acl [2] . The way it is done currently > is translation of the predefined constant to some predefined ACL Provider > [3]. I do not see if we can add something to the current FLIP. I suppose > that eventual extension of the supported ACLProvider would be > straightforward and could be done outside of the current Flip as soon > as concrete use-case requirements arise. > > Kind Regards > Oleksandr > > [1] > https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List)https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List) > <https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List)> > [2] > https://nightlies.apache.org/flink/flink-docs-release-1.18/docs/deployment/config/#high-availability-zookeeper-client-acl > [3] > https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java#L208 > > > > On Mon, Jan 22, 2024 at 1:10 PM Matthias Pohl > <matthias.p...@aiven.io.invalid> wrote: > >> Hi Alex, >> thanks for bringing this up and sorry for jumping into the discussion that >> late. Exposing additional curator configuration parameters through Flink's >> config sounds reasonable. Overall, I am fine with the selection of >> parameters. My judgement is entirely based on the curator >> description, though. If someone has more experience with >> configuring/deploying ZooKeeper; it would be great to get more insights. >> >> Here are a few remarks: >> - You propose to use camel-case parameter names. I tried to find some >> documentation where we decide on the actual formatting for configuration >> parameters. Unfortunately, I didn't find any. The Flink configuration >> appears to follow kebap-casing, though. We might want to use this format >> in >> this FLIP as well. >> >> A bit out-of-scope for this FLIP: Is the configuration parameter format >> documented somewhere? Should we add such a rule to the coding conventions >> [1]. What do others think? >> >> - You mention the authentication parameter which expects byte[]. You >> suggest using getBytes() to transform the value from the Flink >> configuration "Map<String, String>" to what Curator expects. Can you >> elaborate in the FLIP a bit more how the overall configuration would look >> like in the end? I assume you refer to ConfigOptions#mapType when talking >> about Map<String, String>? Do you want to allow multiple AuthInfo records >> to be configurable? >> >> - In [2], I found an example where they also suggest utilizing the >> AclProvider for allowing write access. This wouldn't be necessary in our >> case because the credentials are managed by an outside process, I guess? >> Or >> do we need to look into supporting AclProviders as well? >> >> Best, >> Matthias >> >> [1] >> >> https://flink.apache.org/how-to-contribute/code-style-and-quality-formatting/#naming >> [2] https://stackoverflow.com/a/55171761 >> >> >> On Mon, Jan 8, 2024 at 9:46 AM Alex Nitavsky <alexnitav...@gmail.com> >> wrote: >> >> > Thanks Zhanghao Chen for the feedback. >> > >> > In general right now there is no way to authorise Flink via the Curator >> > framework, which is probably an important feature which is missing. >> Since >> > public configuration change requires a Flip, current Flip is proposing >> to >> > add several more potentially important Curator configurations. >> > >> > Kind regards >> > Oleksandr >> > >> > On Thu, Jan 4, 2024 at 4:57 AM Zhanghao Chen <zhanghao.c...@outlook.com >> > >> > wrote: >> > >> > > Thanks for driving this. I'm not familiar with the listed advanced >> > Curator >> > > configs, but the previous added switch for disabling ensemble tracking >> > [1] >> > > saved us when deploying Flink in a cloud env where ZK can only be >> > > accessible via URLs. That being said, +1 for the overall idea, these >> > > configs may help users in certain scenarios sooner or later. >> > > >> > > [1] https://issues.apache.org/jira/browse/FLINK-31780 >> > > >> > > Best, >> > > Zhanghao Chen >> > > ________________________________ >> > > From: Alex Nitavsky <alexnitav...@gmail.com> >> > > Sent: Thursday, December 14, 2023 21:20 >> > > To: dev@flink.apache.org <dev@flink.apache.org> >> > > Subject: [DISCUSS] FLIP-402: Extend ZooKeeper Curator configurations >> > > >> > > Hi all, >> > > >> > > I would like to start a discussion thread for: *FLIP-402: Extend >> > ZooKeeper >> > > Curator configurations *[1] >> > > >> > > * Problem statement * >> > > Currently Flink misses several Apache Curator configurations, which >> could >> > > be useful for Flink deployment with ZooKeeper as HA provider. >> > > >> > > * Proposed solution * >> > > We have inspected all possible options for Apache Curator and proposed >> > > those which could be valuable for Flink users: >> > > >> > > - high-availability.zookeeper.client.authorization [2] >> > > - high-availability.zookeeper.client.maxCloseWaitMs [3] >> > > - high-availability.zookeeper.client.simulatedSessionExpirationPercent >> > [4] >> > > >> > > The proposed way is to reflect those properties into Flink >> configuration >> > > options for Apache ZooKeeper. >> > > >> > > Looking forward to your feedback and suggestions. >> > > >> > > Kind regards >> > > Oleksandr >> > > >> > > [1] >> > > >> > > >> > >> https://cwiki.apache.org/confluence/display/FLINK/FLIP-402%3A+Extend+ZooKeeper+Curator+configurations >> > > [2] >> > > >> > > >> > >> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.lang.String,byte%5B%5D) >> > > [3] >> > > >> > > >> > >> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#maxCloseWaitMs(int) >> > > [4] >> > > >> > > >> > >> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#simulatedSessionExpirationPercent(int) >> > > >> > >> >
