quarkus-cache interceptors and @NonBlocking methods

732 views
Skip to first unread message

gwe...@gmail.com

unread,
Dec 21, 2021, 8:31:23 AM12/21/21
to Quarkus Development mailing list
Hi everyone,

Rostislav opened an issue earlier today about the following code which throws an IllegalStateException because of a blocked event loop thread:

@NonBlocking
@Path("/value")
public class ReactiveResource {
    @GET
    @CacheInvalidate(cacheName = "mycache"  )
    public void getValue() {
    }
}

This is new in Quarkus 2.6 because of the programmatic caching API merge which changed the returned type for cache invalidation from void to Uni<Void>. As a consequence of that change, the cache interceptors now contain an await() instruction which is causing the issue Rostislav found.

The change from void to Uni<Void> was initially motivated by the possibility that, some day in the future, there could be an additional remote caching provider in the extension. Remote validation would then be lazy thanks to the Uni. But that may never happen. AFAIK there's no short-term or mid-term plan to add another caching provider.

Now, what should we do about this?

1. Keep Uni<Void> in the programmatic Cache interface but use a different method from the interceptors? I created a small draft about this approach: https://github.com/gwenneg/quarkus/pull/198

2. Replace Uni<Void> with void in the Cache interface? That would make lazy remote invalidation impossible with the new signature, which may be completely fine. A deprecation phase would be required of course.

3. Live with the current issue?

4. Something else?

There is an additional and close issue related to this. CacheResultInterceptor also contains two await() instructions. They are not executed if the method annotated with @CacheResult returns a Uni, but they are executed otherwise. It means the following code won't work:

@NonBlocking @Path("/value") public class ReactiveResource { public static final String CACHE_NAME = "mycache"; @GET @CacheResult(cacheName = CACHE_NAME) public int getValue() { return 1; } }

Should that case be treated by default as a blocking call (with build time transformation, maybe a warning and a note in the doc). Or is there a better way to deal with that?

WDYT?

Gwenneg

gwe...@gmail.com

unread,
Dec 21, 2021, 8:33:23 AM12/21/21
to Quarkus Development mailing list
I forgot to add that @CacheResultInterceptor was already blocking before 2.6 and the programmatic caching API. Instead of waiting for a Uni resolution, we waited for a CompletionStage completion.

Martin Kouba

unread,
Dec 21, 2021, 8:43:06 AM12/21/21
to gwe...@gmail.com, Quarkus Development mailing list
On 21. 12. 21 14:31, gwe...@gmail.com wrote:
> Hi everyone,
>
> Rostislav opened an issue
> <https://github.com/quarkusio/quarkus/issues/22431> earlier today about
> the following code which throws an IllegalStateException because of a
> blocked event loop thread:
>
> @NonBlocking
> @Path("/value")
> public class ReactiveResource {
>     @GET

I'm no REST guru but shouldn't this be @POST?

/me is hiding ;-)

>     @CacheInvalidate(cacheName = "mycache"  )
>     public void getValue() {
>     }
> }
>
> This is new in Quarkus 2.6 because of the programmatic caching API merge
> <https://github.com/quarkusio/quarkus/pull/8631> which changed the
> returned type for cache invalidation
> <https://github.com/quarkusio/quarkus/blob/3a9f0dd1d1f3e91aed57f3a781b727101981fb72/extensions/cache/runtime/src/main/java/io/quarkus/cache/Cache.java#L43-L55>
> from void to Uni<Void>. As a consequence of that change, the cache
> interceptors now contain an await() instruction
> <https://github.com/quarkusio/quarkus/blob/3a9f0dd1d1f3e91aed57f3a781b727101981fb72/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/CacheInvalidateInterceptor.java#L39> which
> is causing the issue Rostislav found.

Hm, a stupid question: do we really need to .await().indefinitely()
here? Can't we just request an invalidation and proceed?

>
> The change from void to Uni<Void> was initially motivated by the
> possibility that, some day in the future, there could be an additional
> remote caching provider in the extension. Remote validation would then
> be lazy thanks to the Uni. But that may never happen. AFAIK there's no
> short-term or mid-term plan to add another caching provider.
>
> Now, what should we do about this?
>
> 1. Keep Uni<Void> in the programmatic Cache interface but use a
> different method from the interceptors? I created a small draft about
> this approach: https://github.com/gwenneg/quarkus/pull/198
> <https://github.com/gwenneg/quarkus/pull/198>
>
> 2. Replace Uni<Void> with void in the Cache interface? That would make
> lazy remote invalidation impossible with the new signature, which may be
> completely fine. A deprecation phase would be required of course.
>
> 3. Live with the current issue?
>
> 4. Something else?
>
> There is an additional and close issue related to this.
> CacheResultInterceptor
> <https://github.com/quarkusio/quarkus/blob/main/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/CacheResultInterceptor.java> also
> contains two await() instructions. They are not executed if the method
> annotated with @CacheResult returns a Uni, but they are executed
> otherwise. It means the following code won't work:
>
> @NonBlocking @Path("/value") public class ReactiveResource { public
> static final String CACHE_NAME = "mycache"; @GET @CacheResult(cacheName
> = CACHE_NAME) public int getValue() { return 1; } }
>
> Should that case be treated by default as a blocking call
> <https://quarkus.io/blog/resteasy-reactive-smart-dispatch/#new-world-new-rules>
> (with build time transformation, maybe a warning and a note in the doc).
> Or is there a better way to deal with that?
>
> WDYT?
>
> Gwenneg
>
> --
> You received this message because you are subscribed to the Google
> Groups "Quarkus Development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to quarkus-dev...@googlegroups.com
> <mailto:quarkus-dev...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/quarkus-dev/9bc725a5-7070-49ae-b4c4-b96f4e68b90an%40googlegroups.com
> <https://groups.google.com/d/msgid/quarkus-dev/9bc725a5-7070-49ae-b4c4-b96f4e68b90an%40googlegroups.com?utm_medium=email&utm_source=footer>.

--
Martin Kouba
Software Engineer
Red Hat, Czech Republic

Rostislav Svoboda

unread,
Dec 21, 2021, 8:52:26 AM12/21/21
to mko...@redhat.com, gwe...@gmail.com, Quarkus Development mailing list

On 21. 12. 2021, at 14:42, Martin Kouba <mko...@redhat.com> wrote:

On 21. 12. 21 14:31, gwe...@gmail.com wrote:
Hi everyone,
Rostislav opened an issue <https://github.com/quarkusio/quarkus/issues/22431> earlier today about the following code which throws an IllegalStateException because of a blocked event loop thread:
@NonBlocking
@Path("/value")
public class ReactiveResource {
    @GET

I'm no REST guru but shouldn't this be @POST?

To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/34e413d4-7e5a-b9d1-4d07-bb1bd789fd45%40redhat.com.


Stuart Douglas

unread,
Dec 21, 2021, 9:42:55 PM12/21/21
to gwe...@gmail.com, Quarkus Development mailing list
On Wed, 22 Dec 2021 at 00:31, gwe...@gmail.com <gwe...@gmail.com> wrote:
Hi everyone,

Rostislav opened an issue earlier today about the following code which throws an IllegalStateException because of a blocked event loop thread:

@NonBlocking
@Path("/value")
public class ReactiveResource {
    @GET
    @CacheInvalidate(cacheName = "mycache"  )
    public void getValue() {
    }
}

This is new in Quarkus 2.6 because of the programmatic caching API merge which changed the returned type for cache invalidation from void to Uni<Void>. As a consequence of that change, the cache interceptors now contain an await() instruction which is causing the issue Rostislav found.

The change from void to Uni<Void> was initially motivated by the possibility that, some day in the future, there could be an additional remote caching provider in the extension. Remote validation would then be lazy thanks to the Uni. But that may never happen. AFAIK there's no short-term or mid-term plan to add another caching provider.

Now, what should we do about this?

1. Keep Uni<Void> in the programmatic Cache interface but use a different method from the interceptors? I created a small draft about this approach: https://github.com/gwenneg/quarkus/pull/198

2. Replace Uni<Void> with void in the Cache interface? That would make lazy remote invalidation impossible with the new signature, which may be completely fine. A deprecation phase would be required of course.

3. Live with the current issue?

4. Something else?

I think the invalidation interceptor should be changed to support Uni<Void> as a return type, and then use similar code to the other interceptor (i.e. the code above should still be an error).
 

There is an additional and close issue related to this. CacheResultInterceptor also contains two await() instructions. They are not executed if the method annotated with @CacheResult returns a Uni, but they are executed otherwise. It means the following code won't work:

@NonBlocking @Path("/value") public class ReactiveResource { public static final String CACHE_NAME = "mycache"; @GET @CacheResult(cacheName = CACHE_NAME) public int getValue() { return 1; } }

Should that case be treated by default as a blocking call (with build time transformation, maybe a warning and a note in the doc). Or is there a better way to deal with that?


IMHO the solution should be the same as above, it should return Uni<Integer>.

Stuart
 
WDYT?

Gwenneg

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages