Repository navigation
S3_PING.removeAll: log a failed listing instead of throwing MissingResourceException - #662
Merged
rhusar merged 1 commit intoOct 8, 2026
Conversation
…sourceException
The catch block reported the failure with Util.getMessage("FailedDeletingAllObjects"), a key JGroups'
jg-messages.properties does not have, so the report threw MissingResourceException: the original exception
was lost and the new one escaped removeAll(), which runs from FILE_PING.handleView() during a view change.
Log a formatted message with the exception instead, as readAll() and write() do.
Signed-off-by: rliu3 <ray.liu@toronto.ca>
rayliu2007
force-pushed
the
fix-s3ping-removeall-message
branch
from
October 7, 2026 21:27
9f78776 to
c946acc
Compare
Member
|
Interesting, looks like a remnant when it was moved from main jgroups source. Ill resolve this tomorrow. Thanks for the PR! Letting ci run |
rhusar
approved these changes
Oct 8, 2026
rhusar
left a comment
rhusar
left a comment
Member
There was a problem hiding this comment.
LGTM
However, we really need to remove all the calls to that method as upstream can change and break the project without us even knowing. There is also a more polish that can happen here which is out of scope for this PR.
Member
|
Thank you @rayliu2007! I will add more fixes for this minor and cut a release. |
Member
|
Followup #663 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When the listing in
S3_PING.removeAll()fails (no credentials,AccessDenied, a network error), itscatchblockreported it with
Util.getMessage("FailedDeletingAllObjects"). That key is not in JGroups'jg-messages.properties(5.5.4, 5.5.7 or
master), so the report itself threwMissingResourceException:removeAll(). Withremove_all_data_on_view_change="true"that method runs fromFILE_PING.handleView(), insidedown(VIEW_CHANGE)on the coordinator; we saw it failJChannel.connect().The fix logs a formatted message with the exception, the way
readAll()andwrite()in the same class already do,so the protocol no longer depends on a key in JGroups' bundle.
S3_PINGRemoveAllTestgivesS3_PINGanS3Clientwhose every call fails and checks thatremoveAll()returnsnormally. It fails on the current code with the exception above and passes with the change. It needs neither AWS
credentials nor a container.