2018-04-06 11:11 GMT+02:00 Mark Struberg <strub...@yahoo.de.invalid>:
> > 1. don't expose the read side of the API, hide it under our impls
> > (resolver, @configuration, ...)
>
> We will add the feature to @Configuration. That's on our list and doable
> in a perfectly transparent way.
>
> Regarding ConfigResolver. The whole class mainly exists for backward
> compat reasons.
> The only actively used method should be getConfig(). The Config interface
> is the way to go. Thus the functionality only got added there.
>
Good catch, read it as "everywhere" so it includes config
>
>
> > 2. don't use typeresolvers but keys to consider
>
> That's not possible. You need the whole lookup chain information.
> Suffixes, variable replacement etc.
> You simply don't have this information if you only go by the keys.
> That's why your proposed API cannot work.
>
Nop, everything should end up on a key so it must work (or we have a more
vicious issue).
For @Config we can do it automatically as you explained, for other cases
the user will need to do it anyway himself
so having this indirection or not doesn't help much. == it should be fine
and easier for end user to just check keys.
>
>
> > 3. provide a context handler
>
> What do you understand by that?
>
Don't provide a snapshot which allows to read values but something to
decorate a "task" which will set the correct value reader under the hood.
It avoids to have to propagate the snapshot in the whole user codebase
which is a huge breaking change not that justified technically.
It would also mess up all your code if you think about it and reduce the
composability of the modules of an app at scale.
>
> LieGrue,
> strub
>
>
> > Am 06.04.2018 um 10:34 schrieb Romain Manni-Bucau <rmannibu...@gmail.com
> >:
> >
> > Mark, I started to play with the API you proposed and I'd like to propose
> > this change:
> >
> > 1. don't expose the read side of the API, hide it under our impls
> > (resolver, @configuration, ...)
> > 2. don't use typeresolvers but keys to consider
> > 3. provide a context handler
> >
> > Here is how I would envision it (dont give too much values on the naming)
> >
> > public interface Config { // skipping "old" part
> > FrozenConfigurationValues freeze(String... keys);
> > }
> >
> >
> >
> > public interface FrozenConfigurationValues {
> > <T> T execute(Supplier<T> task);
> > }
> >
> >
> > this part would be in the *api*.
> >
> > Then the impl of FrozenConfigValues would be in -impl module and its
> usage
> > in our resolvers (TypeResolver, ConfigResolvers)+@Configuration which
> would
> > make it transparent for the end user
> > as soon as he wrapped his code execution with the FrozenConfiguration
> > values:
> >
> > final FrozenConfigurationValues fcv = config.freeze("ftp.host",
> > "ftp.port", "ftp.dir", "ftp.user", "ftp.password");
> > fcv.execute(() -> {
> > return *ftp.download(); // uses @Configuration FtpConfiguration*
> > });
> >
> > Advantage is it will allow to use nested "execute" easily and therefore
> > keep the code modular on the user side.
> > We can be fancy and provide a default filter which would take the keys to
> > freeze for each requests in ConfigResolver OOTB if we want but I'm more
> > confident with this kind of design
> > than starting to expose some internals directly.
> >
> > Side note: the fact it provides an execute method is to be compatible
> with
> > reactive programming (async context for servlets for instance) and not
> only
> > synchronous programming.
> >
> > The big gain is the user code does NOT need to be modified to benefit
> from
> > this feature and the setup can be done on top of the app (through a
> filter,
> > interceptor or anything else)
> > which is consistent with the fact you can plug at deploy time some
> sources
> > the dev is not aware off. It means you can dev assuming your config is
> > always consistent (cause
> > you dev with classpath files only) and behave very well at runtime cause
> at
> > deploy time the work has been done without requiring to recode parts of
> the
> > app. In other words
> > we give spaces to dev, devops and ops ;).
> >
> > wdyt?
> >
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <https://www.packtpub.com/application-development/java-
> ee-8-high-performance>
> >
> > 2018-04-06 9:40 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >
> >> ok ok, i'm fine not having it implementing it - just thought it was not
> a
> >> blocker.
> >>
> >> Can we get an unwrap in Config *api* to be able to plug features like
> that
> >> (which allows us to use it under the hood and when we think it is ready
> we
> >> promote it to the api)?
> >>
> >> config.unwrap(SnapshotService.class)....
> >>
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >> <https://rmannibucau.metawerx.net/> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github
> >> <https://github.com/rmannibucau> | LinkedIn
> >> <https://www.linkedin.com/in/rmannibucau> | Book
> >> <https://www.packtpub.com/application-development/java-
> ee-8-high-performance>
> >>
> >> 2018-04-06 9:13 GMT+02:00 Mark Struberg <strub...@yahoo.de.invalid>:
> >>
> >>> But the ConfigTransaction/ConfigSnapshot has only one functionality:
> >>> getValue().
> >>> But the Config itself is the underlying infrastructure holder and has
> >>> tons of features.
> >>>
> >>> Having ConfigSnapshot extends Config and just being a 'frozen' Config
> >>> would mean we would also need to implement all features.
> >>> And of course also resolve ALL conigured values at this time, and not
> >>> only the few given ones.
> >>> I think that is too much.
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>> Am 06.04.2018 um 09:08 schrieb Romain Manni-Bucau <
> >>> rmannibu...@gmail.com>:
> >>>>
> >>>> 2018-04-06 8:51 GMT+02:00 Mark Struberg <strub...@yahoo.de.invalid>:
> >>>>
> >>>>> Btw, ImmutableConfig would kind of suggest that it extends our Config
> >>>>> interface, isn't?
> >>>>>
> >>>>
> >>>> Didn't think to that but it would be possible since we snapshot a
> >>> subconfig
> >>>> which can be a config. A snapshot sounds like it is the same object
> type
> >>>> but "frozen" no?
> >>>>
> >>>> Anyway what about making it an internal of ConfigImpl and not a Config
> >>>> thing, then we don't care much of the name, right?
> >>>>
> >>>>
> >>>>> But that's not the case, it is really something different.
> >>>>>
> >>>>> LieGrue,
> >>>>> strub
> >>>>>
> >>>>>
> >>>>>> Am 05.04.2018 um 22:02 schrieb Romain Manni-Bucau <
> >>> rmannibu...@gmail.com
> >>>>>> :
> >>>>>>
> >>>>>> Between the 3 createSnapshot or snapshot.
> >>>>>>
> >>>>>> What about ImmutableConfig and Config.toImmutable(keys)?
> >>>>>>
> >>>>>> Not a big deal though.
> >>>>>>
> >>>>>>
> >>>>>> Le 5 avr. 2018 21:38, "Thomas Andraschko" <
> >>> andraschko.tho...@gmail.com>
> >>>>> a
> >>>>>> écrit :
> >>>>>>
> >>>>>> +1 for ConfigSnapshot and Config#snapshot
> >>>>>>
> >>>>>> 2018-04-05 21:32 GMT+02:00 Mark Struberg <strub...@yahoo.de.invalid
> >:
> >>>>>>
> >>>>>>> Had a long discussion with Tomas Langer about atomic access this
> >>>>>> afternoon.
> >>>>>>>
> >>>>>>> What we came up with is probably better than 'ConfigTransaction',
> >>>>> because
> >>>>>>> the 'Transaction' term really creates the wrong impression.
> >>>>>>>
> >>>>>>> So what about
> >>>>>>>
> >>>>>>> * renaming ConfigTransaction to ConfigSnapshot and
> >>>>>>> * Config#createTransation to
> >>>>>>> - Config#resolveSnapshot or
> >>>>>>> - Config#createSnapshot, or just
> >>>>>>> - Config#snapshot ?
> >>>>>>>
> >>>>>>> Any thoughts? Or even any better ideas?
> >>>>>>>
> >>>>>>> txs and LieGrue,
> >>>>>>> strub
> >>>>>>>
> >>>>>>>> Am 05.04.2018 um 10:39 schrieb Romain Manni-Bucau <
> >>>>> rmannibu...@gmail.com
> >>>>>>>> :
> >>>>>>>>
> >>>>>>>> Yes, you are right, in all cases the source must send an event to
> >>> the
> >>>>>>>> aggregator (config) to notify it to update its state (timestamp or
> >>>>> not).
> >>>>>>>> Thought we could support it transparently but seems there always
> >>> have
> >>>>>>>> border cases in advanced impl we sadly often rely on in prod ;).
> >>>>>>>> It cant be a config access directly but we can add an update event
> >>> for
> >>>>>>> that
> >>>>>>>> (if we dont want a cdi event we can allow sources to implement
> >>>>> Updatable
> >>>>>>> {
> >>>>>>>> void afterUpdate(Collection<String> keys) } probably which would
> do
> >>> the
> >>>>>>>> relationship without having a real cycle on the code)
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Romain Manni-Bucau
> >>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> >>>>>>> rmannibucau> |
> >>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>> <https://www.packtpub.com/application-development/java-
> >>>>>>> ee-8-high-performance>
> >>>>>>>>
> >>>>>>>> 2018-04-05 10:30 GMT+02:00 Mark Struberg
> <strub...@yahoo.de.invalid
> >>>> :
> >>>>>>>>
> >>>>>>>>> But the writeLock would need to be performed by the
> ConfigSources.
> >>>>>>>>> So we need to change them anyway.
> >>>>>>>>>
> >>>>>>>>> In the current solution any ConfigSource which performans an
> update
> >>>>>>> would
> >>>>>>>>> just need to call the reportAttributeChange callback.
> >>>>>>>>>
> >>>>>>>>> LieGrue,
> >>>>>>>>> strub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Am 05.04.2018 um 09:39 schrieb Romain Manni-Bucau <
> >>>>>>> rmannibu...@gmail.com
> >>>>>>>>>> :
> >>>>>>>>>>
> >>>>>>>>>> 2018-04-05 9:31 GMT+02:00 Mark Struberg
> <strub...@yahoo.de.invalid
> >>>> :
> >>>>>>>>>>
> >>>>>>>>>>> Romain, your mail client is pretty broken. All my original
> >>> content
> >>>>>> and
> >>>>>>>>>>> your reply looks exactly the same.
> >>>>>>>>>>> Please try to fix your reply-to rules ;)
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> This doesnt work for at least two reasons:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. You assume you have request scoped
> >>>>>>>>>>>
> >>>>>>>>>>> Thus the fallback to native resolving in case there is no
> active
> >>>>>>> Request
> >>>>>>>>>>> Context. Means the current status quo.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> RMB: not exactly since you need a workaround instead of no
> >>> workaround
> >>>>>>> at
> >>>>>>>>>> all so the code path would be better without that.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> 2. You assume the tx has tx guarantee
> >>>>>>>>>>>
> >>>>>>>>>>> No, That's where I struggle. The name
> 'ConfigurationTransaction'
> >>> is
> >>>>>>> not
> >>>>>>>>> a
> >>>>>>>>>>> DB or JTA transaction! It's just that you have kind of an
> atomic
> >>>>>>> access
> >>>>>>>>>>> (like in ACID). Look at the code, it's done via optimistic
> >>> locking.
> >>>>>>> It's
> >>>>>>>>>>> kind of a transactional isolation level READ-COMMITTED, but
> done
> >>> on
> >>>>> a
> >>>>>>>>>>> programmatic level.
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> The only way to have 2 is to lock the whole "current" config
> for
> >>>>> the
> >>>>>>>>> read
> >>>>>>>>>>>> time (so a readwritelock for a simple impl) which makes this
> >>>>> request
> >>>>>>>>>>> scope
> >>>>>>>>>>>> useless since you just need to swap the cache value and update
> >>> it
> >>>>> at
> >>>>>>>>> once
> >>>>>>>>>>>> with the lock.
> >>>>>>>>>>>
> >>>>>>>>>>> That would trash our performance. And it would require to start
> >>> the
> >>>>>>> lock
> >>>>>>>>>>> at the beginning of a request, and then release it at the end
> of
> >>> a
> >>>>>>>>> request.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> No
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> Means if you would want to update some value (write-lock) then
> >>> you
> >>>>>>> would
> >>>>>>>>>>> need to wait until all requests are finished (might take some
> >>>>>>> seconds),
> >>>>>>>>>>> BLOCK all new incoming requests (stand-still in the whole app)
> >>> and
> >>>>>>> only
> >>>>>>>>>>> then you could do the update :(
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> We would have:
> >>>>>>>>>>
> >>>>>>>>>> withWriteLock(updateConfig())
> >>>>>>>>>>
> >>>>>>>>>> and
> >>>>>>>>>>
> >>>>>>>>>> withReadLock(updateConfigInstanceCache())
> >>>>>>>>>>
> >>>>>>>>>> It means the overhead is null everytime except until there is a
> >>> write
> >>>>>>> in
> >>>>>>>>>> the config which is the only way to guarantee the atomicity of
> the
> >>>>>>>>> updates
> >>>>>>>>>> (switch to another ftp/http/... server typically).
> >>>>>>>>>>
> >>>>>>>>>> You can think to a lock per key*s* but it doesnt work cause we
> >>>>> support
> >>>>>>>>>> placeholding and this means you must lock the whole config to
> impl
> >>>>> it.
> >>>>>>>>>>
> >>>>>>>>>> This impl is good in terms of perf since the updates will not be
> >>> that
> >>>>>>>>>> frequent. If you care of that small "stop the world' - but keep
> it
> >>>>>> mind
> >>>>>>>>> it
> >>>>>>>>>> should be very small - then we can use a queue to update the
> data
> >>> so
> >>>>>>> the
> >>>>>>>>>> write fills a queue (or just a flag) which once in the right
> state
> >>>>>> will
> >>>>>>>>>> force the consumers (config instances) to reevaluate the current
> >>>>>>> values.
> >>>>>>>>>> The issue here is that you still need to lock the world to copy
> >>> the
> >>>>>>>>> config
> >>>>>>>>>> state (you cant assume it is scannable in most of the cases) to
> >>> stay
> >>>>>>>>> atomic.
> >>>>>>>>>>
> >>>>>>>>>> Long story short: there is no real choice if you want to be
> atomic
> >>>>>>>>>> transparently (ie without adding a relationship between keys +
> >>> keep
> >>>>> in
> >>>>>>>>> the
> >>>>>>>>>> instance this relationship to know what to update when there is
> a
> >>>>>> write
> >>>>>>>>>> which goes to the placeholding replacements).
> >>>>>>>>>> To already have use RW for that I don't see it as a perf
> blocker.
> >>>>>>>>>>
> >>>>>>>>>> What about trying and measure it?
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> LieGrue,
> >>>>>>>>>>> strub
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> Am 04.04.2018 um 23:55 schrieb Romain Manni-Bucau <
> >>>>>>>>> rmannibu...@gmail.com
> >>>>>>>>>>>> :
> >>>>>>>>>>>>
> >>>>>>>>>>>> Le 4 avr. 2018 19:37, "Mark Struberg"
> <strub...@yahoo.de.invalid
> >>>>
> >>>>> a
> >>>>>>>>>>> écrit :
> >>>>>>>>>>>>
> >>>>>>>>>>>> But that's still problematic.
> >>>>>>>>>>>> you have request1 ongoing and call in the following order:
> >>>>>>>>>>>> ftpConfig.host();ftpConfig.port();// <- and here some config
> >>>>> update
> >>>>>>>>>>> happens
> >>>>>>>>>>>> ftpConfig.username();
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> So even if we update the whole ftpConfig 'at once' you will
> end
> >>> up
> >>>>>>> with
> >>>>>>>>>>>> mixed up information in this request, right?
> >>>>>>>>>>>> The only viable solution is to have a @RequestScoped
> >>>>>>> ConfigTransaction
> >>>>>>>>>>>> spanning all those TypedResolvers of the whole @Configuration.
> >>> At
> >>>>>>>>> least I
> >>>>>>>>>>>> could not find any better solution.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> This doesnt work for at least two reasons:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. You assume you have request scoped
> >>>>>>>>>>>> 2. You assume the tx has tx guarantee
> >>>>>>>>>>>>
> >>>>>>>>>>>> The only way to have 2 is to lock the whole "current" config
> for
> >>>>> the
> >>>>>>>>> read
> >>>>>>>>>>>> time (so a readwritelock for a simple impl) which makes this
> >>>>> request
> >>>>>>>>>>> scope
> >>>>>>>>>>>> useless since you just need to swap the cache value and update
> >>> it
> >>>>> at
> >>>>>>>>> once
> >>>>>>>>>>>> with the lock.
> >>>>>>>>>>>>
> >>>>>>>>>>>> LieGrue,strub
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Wednesday, 4 April 2018, 18:44:13 CEST, Romain Manni-Bucau
> <
> >>>>>>>>>>>> rmannibu...@gmail.com> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Since the cache is per instance we should just clear it on
> >>> eviction
> >>>>>>> at
> >>>>>>>>>>> once
> >>>>>>>>>>>> IMHO
> >>>>>>>>>>>> the issue is: do you want to populate it at once too? tempted
> to
> >>>>> say
> >>>>>>>>> yes
> >>>>>>>>>>>>
> >>>>>>>>>>>> this means it can always be active but requires to be able to
> >>> copy
> >>>>>>> the
> >>>>>>>>>>>> current config state or prevent *any* update while populating
> >>> such
> >>>>>>>>>>> "cache"
> >>>>>>>>>>>>
> >>>>>>>>>>>> +1 to do it without any flag
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Romain Manni-Bucau
> >>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github <
> >>> https://github.com/
> >>>>>>>>>>> rmannibucau>
> >>>>>>>>>>>> |
> >>>>>>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>>>>>> <https://www.packtpub.com/application-development/java-
> >>>>>>>>>>> ee-8-high-performance
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> 2018-04-04 18:40 GMT+02:00 Mark Struberg
> >>> <strub...@yahoo.de.invalid
> >>>>>>>> :
> >>>>>>>>>>>>
> >>>>>>>>>>>>> We should also enhance the support to include @Configuration.
> >>>>>>>>>>>>> e.g. if you have some class like
> >>>>>>>>>>>>> @Configuration(cacheFor=60, cacheUnit=TimeUnit.MINUTES)
> >>>>>>>>>>>>> public class FtpConfigation { String host(); Integer
> port();
> >>>>>>> String
> >>>>>>>>>>>>> username(); String encryptedPwd();}
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Then you will likely resolve all those values in an atomic
> way.
> >>>>>> This
> >>>>>>>>>>> means
> >>>>>>>>>>>>> that the values are basically backed by a @RequestScoped
> >>>>>>>>>>> ConfigTransaction
> >>>>>>>>>>>>> holder.
> >>>>>>>>>>>>> Do we _always_ like to activate this feature?Or do we like to
> >>>>>>>>> introduce
> >>>>>>>>>>>>> another flag in the @Configuration annotation?
> >>>>>>>>>>>>> What about threads which have no request Context
> active?Should
> >>> it
> >>>>>>>>>>>>> automatically fallback to on-demand resolving?
> >>>>>>>>>>>>> LieGrue,strub
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Wednesday, 4 April 2018, 18:08:09 CEST, Mark Struberg
> >>>>>>>>>>>>> <strub...@yahoo.de.INVALID> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> hi folks!
> >>>>>>>>>>>>> please review the proposed solution for DELTASPIKE-1335
> >>>>>>>>>>>>> https://issues.apache.org/jira/browse/DELTASPIKE-1335
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Not quite sure about startTransaction and ConfigTransation
> are
> >>> the
> >>>>>>>>> right
> >>>>>>>>>>>>> terms. Happy to get feedback!
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> txs and LieGrue,strub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
>
>
