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

Remove legacy CountdownLatch #15538

Merged
merged 1 commit into from Sep 19, 2019
Merged

Remove legacy CountdownLatch #15538

merged 1 commit into from Sep 19, 2019

Conversation

@mdogan
Copy link
Member

mdogan commented Sep 11, 2019

Removed legacy CountdownLatch implementation and HazelcastInstance.getCountdownLatch(..) method. Instead we provide unsafe mode for all CP data structures (See #15184).

EE: hazelcast/hazelcast-enterprise#3155

See TDD: https://hazelcast.atlassian.net/wiki/spaces/EN/pages/1749057609/Concurrency+API+Clean-up+Design

Protocol change: hazelcast/hazelcast-client-protocol#225

@mdogan mdogan requested a review from hazelcast/clients as a code owner Sep 11, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-latch branch from 25fa161 to 08ab0bd Sep 11, 2019
@mdogan mdogan added this to the 4.0 milestone Sep 11, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-latch branch 3 times, most recently from de3aa4b to 2ae3630 Sep 11, 2019
@asimarslan asimarslan self-requested a review Sep 16, 2019
@asimarslan

This comment has been minimized.

Copy link
Member

asimarslan commented Sep 16, 2019

@mdogan Can you send the client protocol changes to protocol repo too?

@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 17, 2019

@asimarslan: I'm planning to send changes to the protocol repo. But I want to do that as the final step, since after ICountDownLatch and ISemaphore, I will cleanup IAtomicLong, IAtomicReference and ILock too.

Last week I confirmed this with @mdumandag, he said that protocol and Hazelcast repo are not dependent to each other, so it's possible to do protocol changes after removing message-tasks and codecs.

Are you ok with this?

@mdogan mdogan mentioned this pull request Sep 17, 2019
@asimarslan

This comment has been minimized.

Copy link
Member

asimarslan commented Sep 17, 2019

We removed the dependency for better development experience. We don't need to make protocol releases anymore but I think we still need to review the protocol PR before merging this PR.

@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 17, 2019

@mmedenjak mmedenjak self-requested a review Sep 17, 2019
Copy link
Contributor

ihsandemir left a comment

Codecs are manually modified and hence the generated message type ids are different.

I suggest that you regenerate the codecs instead of manual modification. But I realized that the protocol PR includes the other PR modifications for Semaphore and such and hence it may not be easy to separately generate just for CountdownLatch. This can be done: Generate it and revert the unrelated files changes to this PR such as Semaphore related ones. The current ids may be OK for this PR but whoever submits the next PR with protocol codec changes will have to do this is change anyway when he re-regenerates the codecs.

@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 19, 2019

@hazelcast/clients: Hunting down consistency bugs are easier than changing client related code.🤦‍♂

@mdogan mdogan force-pushed the mdogan:remove-legacy-latch branch from 2ae3630 to 8bcec40 Sep 19, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-latch branch from 8bcec40 to 531efaf Sep 19, 2019
@mdogan mdogan merged commit 8c0ead7 into hazelcast:master Sep 19, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@mdogan mdogan deleted the mdogan:remove-legacy-latch branch Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.