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

Separate statistic for IMap.set(K, V) and IMap.put(K, V) operations #14811

Merged

Conversation

@galibey
Copy link
Contributor

galibey commented Apr 2, 2019

Fixes: #14663

Operations count and latencies will be counted separately for IMap.set
and IMap.put operations. New stats will be available via LocalMapStats.

Fixes: #14663

Operations count and latencies will be counted separately for IMap.set
and IMap.put operations. New stats will be available via LocalMapStats.
…feature/3.13/14663-separate-set-put-stats
@taburet
taburet approved these changes Apr 2, 2019
Copy link
Contributor

taburet left a comment

Looks good, well done, just a single minor comment.

Copy link
Contributor

mustafaiman left a comment

LGTM with one minor comment

}

void checkCompatibility() {
if (nodeEngine.getClusterService().getClusterVersion().isLessThan(Versions.V3_11)) {

This comment has been minimized.

Copy link
@mustafaiman

mustafaiman Apr 2, 2019

Contributor

Rolling upgrade from older than 1 previous version is not supported. Cluster version cannot be less than 3.11. There is no need for this check.

This comment has been minimized.

Copy link
@galibey

galibey Apr 2, 2019

Author Contributor

Ok. Removed it. Thanks.


import java.security.Permission;

public abstract class AbstractMapSetMessageTask<P> extends AbstractMapPartitionMessageTask<P> {

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Apr 2, 2019

Member

can't we have same functionality by introducing a flag? when operation is put set it to true, otherwise false. By using this flag, we can get rid of this class and can use AbstractMapPutMessageTask instead?

This comment has been minimized.

Copy link
@galibey

galibey Apr 3, 2019

Author Contributor

In this case we bring details of sublclasses to the AbstractMapPutMessageTask's level that there are two groups map put tasks, one with flag=true, other one with flag=false. And, we will have to force all subclasses to choose the correct flag. Also, potentially, in the future, we might want to separate stats calculation for "replace" operation too (now it is counted as "put"), in that case a boolean flag won't be enough.
So, summarizing all above, I think the logic separation by one more abstracted class is preferable here. What do you think?

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Apr 3, 2019

Member

ok it makes sense.

Copy link
Member

ahmetmircik left a comment

looks good, only one minor comment.

@galibey galibey merged commit 0876746 into hazelcast:master Apr 5, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@mmedenjak mmedenjak added this to the 4.0 milestone Apr 17, 2019
@galibey galibey deleted the galibey:feature/3.13/14663-separate-set-put-stats branch May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.