Hi all, Thanks for the discussion!
I will update the FLIP to reflect: 1) I will reuse the core option parallelism.default for the parallelism, and 2) I will remove the dynamic properties from the options. And I will open a voting thread. Cheers, Kostas On Tue, Oct 22, 2019 at 3:12 PM Aljoscha Krettek <aljos...@apache.org> wrote: > > Re 1. I think the parallelism setting doesn’t to what was originally planned > or expected anymore. As far as I can see it’s only used on the client side. > There we only check if the user gave a parallelism using “-p”, if they didn’t > we set what is configured under parallelism.default. I’m fine with either > reusing the existing setting or adding a new one, but I gravitate towards > adding a new one that has a better name (possibly, with specifying > parallelism.default as the “deprecated key” for that). > > Re 2. The “encoded” properties should be removed. I’m afraid we have to keep > “attached” and “sae” for now in order to not break existing client behaviour. > But we could also make them hidden/internal options. I’m torn on this one and > gravitate towards keeping them as config values but documenting them as > experimental and clearly describing what they do and imply. > > Best, > Aljoscha > > > On 22. Oct 2019, at 14:26, Kostas Kloudas <kklou...@gmail.com> wrote: > > > > Hi Dawid, > > > > For the first comment, I am also up for re-using as many options as > > possible. My only concern though is that so far the -p and the > > parallelism.default were considered "different" and I cannot tell the > > exact reason. In addition, the "parallelism.default" is not the most > > descriptive name in my opinion. So just for caution, I would go with > > introducing a new option and if we all agree, I would actually > > deprecate the old one, as that is the one with the problematic name. > > > > For the second comment, you are right for the dynamic properties and I > > will remove them from the document. For the rest (i.e. the "shutdown > > after exit" and the "attached"), I do not think we can remove them as > > the user should be allowed to specify them in the configuration. > > > > I am hoping to have more opinions on this, and I will open a voting > > thread as soon as we reach a consensus. > > Cheers, > > Kostas > > > > On Tue, Oct 22, 2019 at 10:49 AM Dawid Wysakowicz > > <dwysakow...@apache.org> wrote: > >> > >> Hi, > >> > >> I really like the idea of unifying all the ways to set a configuration > >> option and its keys. I think it nicely aligns with FLIP-59. > >> > >> I have two comments though. > >> > >> 1. I wonder what are you thoughts on reusing the "parallelism.default" > >> (CoreOptions.DEFAULT_PARALLELISM) instead of introducing a new key > >> "execution.parallelism". I am not sure if we should introduce another > >> key as it might be confusing for users to have two separate config > >> options for client and cluster side. > >> > >> Moreover if we go with a new config option for the client side with a > >> default value of 1, it makes the cluster side option unusable. Right now > >> the logic is that if we set the parallelism to -1(the default value) on > >> the client side, the cluster side configuration is used. > >> > >> Would be interested in an opinion of others on that matter. I think we > >> have few other cases of options that we set on the client side with a > >> fallback on the cluster side. I think we should have a common approach > >> to such scenarios. > >> > >> 2. If I understood it correctly the "dynamic-properties" (maybe also the > >> "attached" and "shutdown-on-exit") are an internal options that should > >> never be set manually by the user through the config file. I think they > >> are internal and should not be exposed. I wonder if we can make them > >> less visible somehow. Also I think we do not need to agree on them > >> through the FLIP process. > >> > >> Best, > >> > >> Dawid > >> > >> On 22/10/2019 10:05, Aljoscha Krettek wrote: > >>> I think this is good to go to a VOTE. We should, however, make it more > >>> prominent that we don’t like the “attached”, “shutdown-on-attached-exit”, > >>> and “dynamic-properties” and that we only add them for backwards > >>> compatibility. > >>> > >>> Best, > >>> Aljoscha > >>> > >>>> On 21. Oct 2019, at 17:48, Kostas Kloudas <kklou...@apache.org> wrote: > >>>> > >>>> Hi all, > >>>> > >>>> As part of FLIP-73 (the Executors effort), we would like to introduce > >>>> some new configuration options. These are needed in order to be able > >>>> to map all the options that the user can specify either > >>>> programmatically or through the CLI into configuration options. > >>>> > >>>> The bylaws specify that every user-facing change should pass through a > >>>> FLIP, so the FLIP with more details on the new options can be found > >>>> here: > >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=133631524 > >>>> > >>>> Please keep the discussion in the mailing list. > >>>> > >>>> Thanks, > >>>> Kostas > >> >
