[ 
https://issues.apache.org/jira/browse/CASSANDRA-21696?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michael Semb Wever updated CASSANDRA-21696:
-------------------------------------------
    Description: 
{{RequestCallbacks.addWithExpiration}} registers a callback whatever the state 
of the reaper executor. A request that reaches the coordinator after 
{{MessagingService.shutdown}} has called {{shutdownNow}} leaves an entry that 
nothing expires, because the reaper is already shut down, so the entry holds 
its message and its response handler for the life of the process. 
CASSANDRA-6948 repaired the same collection of callbacks for the 2.0 expiring 
map, when the reaper could not be restarted after bootstrap or replace.

The graceful path is worse, because it reschedules itself on the same executor 
while the map is not empty:

{code:java}
void shutdownGracefully()
{
    expire();
    if (!callbacks.isEmpty())
        executor.schedule(this::shutdownGracefully, 100L, MILLISECONDS);
    else
        executor.shutdownNow();
}
{code}

A steady arrival of new requests keeps the map populated, so the reaper is 
never shut down, {{awaitTerminationUntil}} throws {{TimeoutException}}, and 
{{MessagingService.shutdown}} waits out its whole deadline before it reports 
the timeout.

The patch sets a flag in both shutdown methods, and each {{addWithExpiration}} 
overload then reports the late request at trace level and registers nothing. A 
caller is unaffected: the message is not sent either, so the caller ends on its 
own deadline, as it does today for a callback the reaper expires.

Patch: 
[mck/CASSANDRA-21696/5.0|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21696/5.0]
Provenance: 
[8aa35c6cab|https://github.com/datastax/cassandra/commit/8aa35c6cab8ccd0ce4326fc60f50c2f9c02e4108]
 by [~mike_tr_adamson]. That commit carries the same flag and guards; this 
patch names the message id and the peer in the trace line and adds the 
regression test the commit lacks.

  was:
{{RequestCallbacks.addWithExpiration}} registers a callback whatever the state 
of the reaper executor. A request that reaches the coordinator after 
{{MessagingService.shutdown}} has called {{shutdownNow}} leaves an entry that 
nothing expires, because the reaper is already shut down, so the entry holds 
its message and its response handler for the life of the process. 
CASSANDRA-6948 repaired the same collection of callbacks for the 2.0 expiring 
map, when the reaper could not be restarted after bootstrap or replace.

The graceful path is worse, because it reschedules itself on the same executor 
while the map is not empty:

{code:java}
void shutdownGracefully()
{
    expire();
    if (!callbacks.isEmpty())
        executor.schedule(this::shutdownGracefully, 100L, MILLISECONDS);
    else
        executor.shutdownNow();
}
{code}

A steady arrival of new requests keeps the map populated, so the reaper is 
never shut down, {{awaitTerminationUntil}} throws {{TimeoutException}}, and 
{{MessagingService.shutdown}} waits out its whole deadline before it reports 
the timeout.

The patch sets a flag in both shutdown methods, and each {{addWithExpiration}} 
overload then reports the late request at trace level and registers nothing. A 
caller is unaffected: the message is not sent either, so the caller ends on its 
own deadline, as it does today for a callback the reaper expires.

Patch: 
[mck/upstream/callbacks-after-shutdown/5.0|https://github.com/thelastpickle/cassandra/tree/mck/upstream/callbacks-after-shutdown/5.0]
Provenance: 
[8aa35c6cab|https://github.com/datastax/cassandra/commit/8aa35c6cab8ccd0ce4326fc60f50c2f9c02e4108]
 by [~mike_tr_adamson]. That commit carries the same flag and guards; this 
patch names the message id and the peer in the trace line and adds the 
regression test the commit lacks.


> Request callbacks registered after messaging service shutdown are never 
> expired
> -------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-21696
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-21696
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Messaging/Internode
>            Reporter: Michael Semb Wever
>            Priority: Normal
>             Fix For: 5.0.x, 6.0.x, 7.x
>
>
> {{RequestCallbacks.addWithExpiration}} registers a callback whatever the 
> state of the reaper executor. A request that reaches the coordinator after 
> {{MessagingService.shutdown}} has called {{shutdownNow}} leaves an entry that 
> nothing expires, because the reaper is already shut down, so the entry holds 
> its message and its response handler for the life of the process. 
> CASSANDRA-6948 repaired the same collection of callbacks for the 2.0 expiring 
> map, when the reaper could not be restarted after bootstrap or replace.
> The graceful path is worse, because it reschedules itself on the same 
> executor while the map is not empty:
> {code:java}
> void shutdownGracefully()
> {
>     expire();
>     if (!callbacks.isEmpty())
>         executor.schedule(this::shutdownGracefully, 100L, MILLISECONDS);
>     else
>         executor.shutdownNow();
> }
> {code}
> A steady arrival of new requests keeps the map populated, so the reaper is 
> never shut down, {{awaitTerminationUntil}} throws {{TimeoutException}}, and 
> {{MessagingService.shutdown}} waits out its whole deadline before it reports 
> the timeout.
> The patch sets a flag in both shutdown methods, and each 
> {{addWithExpiration}} overload then reports the late request at trace level 
> and registers nothing. A caller is unaffected: the message is not sent 
> either, so the caller ends on its own deadline, as it does today for a 
> callback the reaper expires.
> Patch: 
> [mck/CASSANDRA-21696/5.0|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21696/5.0]
> Provenance: 
> [8aa35c6cab|https://github.com/datastax/cassandra/commit/8aa35c6cab8ccd0ce4326fc60f50c2f9c02e4108]
>  by [~mike_tr_adamson]. That commit carries the same flag and guards; this 
> patch names the message id and the peer in the trace line and adds the 
> regression test the commit lacks.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to