ARTEMIS-5002 AMQP producer not unblock if the disk space is freed #5160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gaohoward
force-pushed
the
g_send_unblock_9338
branch
from
8f09f76
to
fec502a
Compare
gaohoward
changed the title
gaohoward
force-pushed
the
g_send_unblock_9338
branch
from
fec502a
to
15135f0
Compare
|
@clebertsuconic can you take a look of this PR see if the fix makes sense? Thanks! |
gemmellr
reviewed
Aug 20, 2024
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
...tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AMQPBlockingTest.java
Outdated
Show resolved
Hide resolved
gaohoward
force-pushed
the
g_send_unblock_9338
branch
from
15135f0
to
4dd3911
Compare
|
@clebertsuconic @gemmellr I've updated the PR. Thanks! |
gemmellr
reviewed
Aug 21, 2024
...st/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullFailPolicyTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullFailPolicyTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullFailPolicyTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullFailPolicyTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullFailPolicyTest.java
Outdated
Show resolved
Hide resolved
| @@ -1096,6 +1096,7 @@ public boolean checkMemory(boolean runOnFailure, Runnable runWhenAvailableParame | |||
| if (isFull()) { | |||
| if (runOnFailure && runWhenAvailable != null) { | |||
| addToBlockList(runWhenAvailable, blockedCallback); | |||
| pagingManager.addBlockedStore(this); | |||
There was a problem hiding this comment.
The use of this same call lower down the class is gated by `if (usingGlobalMaxSize || pagingManager.isDiskFull()) {``, does this need to be?
It is also surrounded by various other double-checks around racing being not-full again, it seems hard to reconcile that path needing it but this one not.
I cant quite wrap my head around this so think @clebertsuconic might need to chip in here.
There was a problem hiding this comment.
me neither. Need @clebertsuconic's expertise. :)
There was a problem hiding this comment.
PR updated. The code path is really processing many conditions, not sure what's impact of that.
gaohoward
force-pushed
the
g_send_unblock_9338
branch
2 times, most recently
from
aa007e1
to
414b1b9
Compare
gemmellr
reviewed
Aug 26, 2024
...sts/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullTest.java
Outdated
Show resolved
Hide resolved
...sts/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/GlobalDiskFullTest.java
Outdated
Show resolved
Hide resolved
gaohoward
force-pushed
the
g_send_unblock_9338
branch
from
414b1b9
to
0862587
Compare
gaohoward
force-pushed
the
g_send_unblock_9338
branch
3 times, most recently
from
94fa2a3
to
ac182a7
Compare
gaohoward
force-pushed
the
g_send_unblock_9338
branch
from
ac182a7
to
1428fc6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(test adapted from the one on the Jira)