Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match Map.putAll with Map.put behavior upon map listener attribute type mismatch #16144

Merged

Conversation

@tkountis
Copy link
Contributor

tkountis commented Nov 28, 2019

Map.putAll differs from a plain Map.put(), because it triggers the listener events through the main run() block, which propagates exceptions to caller. Map.put(), does that as part of the afterRun() which only logs the error.

Fixes #15415

@tkountis tkountis added this to the 4.0 milestone Nov 28, 2019
@tkountis tkountis requested a review from ahmetmircik Nov 28, 2019
@tkountis tkountis self-assigned this Nov 28, 2019
@vojtechtoman vojtechtoman self-requested a review Nov 28, 2019
// This should happen in after-run to avoid surfacing exceptions during predicate attribute conversion.
// Single Map.put() behaves the same. see. https://github.com/hazelcast/hazelcast/issues/15415
for (EventDescriptor descriptor : unpublishedEvents) {
mapEventPublisher.publishEvent(descriptor.caller, descriptor.mapName,

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 29, 2019

Member

Can we catch and log exceptions inside publishEvent method of MapEventPublisherImpl?

Reasonings:

  1. Centralize place for all map operations, since some other map operations have still same issue.
  2. We can catch exceptions per listener basis to minimize blast of possible failure. For instance, we have attached 100 listener to map but only one of them is throwing exceptions, this failure shouldn't block event flow to other listener.

This comment has been minimized.

Copy link
@tkountis

tkountis Dec 2, 2019

Author Contributor

@ahmetmircik followed your recommendation. It seemed a bit more risky, but I agree, the API is not perfect to handle errors, when it comes to bulk ops like this, one failure shouldn't block the rest.

static class EventDescriptor {

private final Address caller;
private final String mapName;

This comment has been minimized.

Copy link
@vojtechtoman

vojtechtoman Nov 29, 2019

Contributor

Minor: caller and mapName are not really necessary as those can be obtained from the operation.

@tkountis tkountis force-pushed the tkountis:fix/4.0/map-listener-exceptions-putall branch 2 times, most recently from 5f06e53 to 88a41bf Dec 2, 2019
@tkountis tkountis force-pushed the tkountis:fix/4.0/map-listener-exceptions-putall branch from 88a41bf to 7d65b30 Dec 2, 2019
@tkountis tkountis merged commit 6228295 into hazelcast:master Dec 2, 2019
1 check passed
1 check passed
default Test PASSed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.