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 <[email protected]>: > > 2018-04-06 8:51 GMT+02:00 Mark Struberg <[email protected]>: > >> 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 <[email protected] >>> : >>> >>> 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" <[email protected]> >> a >>> écrit : >>> >>> +1 for ConfigSnapshot and Config#snapshot >>> >>> 2018-04-05 21:32 GMT+02:00 Mark Struberg <[email protected]>: >>> >>>> 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 < >> [email protected] >>>>> : >>>>> >>>>> 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 <[email protected]>: >>>>> >>>>>> 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 < >>>> [email protected] >>>>>>> : >>>>>>> >>>>>>> 2018-04-05 9:31 GMT+02:00 Mark Struberg <[email protected]>: >>>>>>> >>>>>>>> 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 < >>>>>> [email protected] >>>>>>>>> : >>>>>>>>> >>>>>>>>> Le 4 avr. 2018 19:37, "Mark Struberg" <[email protected]> >> 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 < >>>>>>>>> [email protected]> 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 <[email protected] >>>>> : >>>>>>>>> >>>>>>>>>> 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 >>>>>>>>>> <[email protected]> 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 >>>>>> >>>>>> >>>> >>>> >> >>
