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 > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
