@Matthias Thanks for raising the question. AFAIK, there is no guide of this naming convention yet. But +1 to add this naming convention in Flink code style guide. So that new configuration names can follow the guide.
However, I tend to not force the configuration name alignment in Flink 2.0. It does not bring obvious benefits to users but will increase the migration cost. And the feature freeze of 1.19 is coming soon. I think we can add aligned key names for those exceptional config options in 1.20, but remove the old keys in later major versions. Thanks, Zhu Matthias Pohl <matthias.p...@aiven.io> 于2024年1月23日周二 19:45写道： > - 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) >>> > > >>> > >>> >>
