quarkus
quarkus copied to clipboard
Use the new Vertx local context storage
This is to keep up with the Vertx API changes for https://github.com/eclipse-vertx/vert.x/commit/88cf77bfe4f44c3dc5ee89d65b8891674088de46, although I'm not very happy with the current state (that's why is in draft):
VertxLocalsHelper.is "stealthy" using the new API by intercepting when users are passingContextLocal-like keys, which means performing 2 instanceof checks (one being an interface check too, which is not yet safe with JDK till 21...)- As written in the
TODOparts, we have some ordering issue: we need the keys statically defined asACCESS_TOGGLE_KEYto be initialized before instantiating theVertxinstance has to use them, due to https://github.com/eclipse-vertx/vert.x/blob/4.5.7/src/main/java/io/vertx/core/impl/VertxImpl.java#L194 which is forcing every new context creation to dedicate the fixed amount of "optimized" keys in the context's local storage - The new
AccessModeI've defined is good enough that having a new method inAccessMode::putRelease, would have been better IMO (@vietj )
For @mkouba and @Ladicek : the new "optimized" API for local storage is double edge sword because it needs some care on init ordering, need some care in dev mode (I think, @geoand ? @sanne ?) and requires to create local "permanent" keys only for the ones we're 100% are going to be stored on each context (and I need some help from @Ladicek and @mkouba for such, I have no IDEA!)
Additionally there are some weird things in the original API (but is ok, the purpose of dog fooding is indeed to try things and found how to use them!): @vietj see https://github.com/quarkusio/quarkus/compare/main...franz1981:quarkus:local_ctx?expand=1#diff-af571e338cf42e877b9b99b70840f0842b293365f18c24d5fb24c930e5b0b064R64: this one is just wrong, because we cannot query all the local storage ContextLocal's values stored in the source context...we cannot do it unless we have a new method as ContextInternal::addLocals(ContextInternal from, AccessMode accessMode)
PTAL @mariofusco @mkouba
Note: I'm still concerned about the ordering by which these statically defined "keys" are generated, in order to happen before Vertx is created: any suggestion on how to do it the quarkus-way?
In order to enforce the ordering of key creation I should make a vertx provider which is look-up before Vertx instance is allocated, see doc at https://github.com/eclipse-vertx/vert.x/blob/4.5.7/src/main/java/io/vertx/core/spi/context/storage/ContextLocal.java#L25C100-L25C120
This PR will remain in draft till https://github.com/eclipse-vertx/vert.x/pull/5215 is going to be into Vertx 4.x because ATM we have no way to duplicate "optimized" keys while creating a new duplicate context.
This PR is still blocked by the mentioned https://github.com/eclipse-vertx/vert.x/pull/5215 one, which should be ported to 4.x as well.
@cescoffier https://github.com/eclipse-vertx/vertx-grpc/blob/main/vertx-grpc-context-storage/src/main/java/io/grpc/override/ContextStorageOverride.java#L27 probably I need to send a PR just to fix this which is should be broken in master
@franz1981 should we go forward with this, or should it wait Vertx 5?
Hi @cescoffier I would love to move this forward but there is a missing "feature" in vertx 4.x i.e. moving local storage values from duplicated ctx with a filter - or just been able to query them. In an hour I will attach the relevant issue which explain this, because the same applies to vertx 5: if we want to use the new API we need it on vertx
@franz1981 do you have the relevant issues?
@cescoffier I cannot see an issue but https://github.com/eclipse-vertx/vert.x/pull/5215 on vertx side which is the in progress work on it. IDK if Julien is aware of exactly we need