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 AtomicLong impl and IdGenerator #15601

Merged
merged 1 commit into from Sep 26, 2019

Conversation

@mdogan
Copy link
Member

mdogan commented Sep 24, 2019

Additonally removed deprecated IdGenerator.

Moved old AtomicLong impl to an internal package
and renamed it to LongRegister. LongRegister still uses
the old service name to make it backward compatible
while doing perf testing.
LongRegister is using 255 (max id) as codec id.

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

EE: hazelcast/hazelcast-enterprise#3174
Protocol change: hazelcast/hazelcast-client-protocol#231

@mdogan mdogan added this to the 4.0 milestone Sep 24, 2019
@mdogan mdogan requested a review from hazelcast/clients as a code owner Sep 24, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-atomiclong branch from adcb1ac to d79111d Sep 24, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-atomiclong branch from d79111d to dd90b50 Sep 24, 2019
@mdogan mdogan changed the title Remove legacy AtomicLong impl Remove legacy AtomicLong impl and IdGenerator Sep 24, 2019
@mdogan mdogan force-pushed the mdogan:remove-legacy-atomiclong branch from dd90b50 to c8a08e1 Sep 24, 2019
@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 24, 2019

run-lab-run

@pveentjer pveentjer self-requested a review Sep 25, 2019
@pveentjer

This comment has been minimized.

Copy link
Member

pveentjer commented Sep 25, 2019

This PR is going to lead to very big problems to me. Because I will not be able to go to previous version of the 4.0 branch.

@pveentjer

This comment has been minimized.

Copy link
Member

pveentjer commented Sep 25, 2019

I don't care for the Id generator; fine to delete it. But moving of IAtomicLong will give me very big headache.

@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 25, 2019

@pveentjer: As we talked before, you will be able to use previous versions. You just need to change access to the IAtomicLong. Instead of HazelcastInstace.getAtomicLong(name), you will use HazelcastInstance.getDistributedObject("hz:impl:atomicLongService", name).

@pveentjer

This comment has been minimized.

Copy link
Member

pveentjer commented Sep 25, 2019

The package name of IAtomicLong will change.. right?

HazelcastInstance local = instances[0];
HazelcastInstance target = instances[instances.length - 1];
String name = generateKeyOwnedBy(target);
longRegister = local.getDistributedObject(LongRegisterService.SERVICE_NAME, name);

This comment has been minimized.

Copy link
@mdogan

mdogan Sep 25, 2019

Author Member

@pveentjer: see 👆

@mdogan

This comment has been minimized.

Copy link
Member Author

mdogan commented Sep 25, 2019

Since #15143, IAtomicLong is in com.hazelcast.cp package, it will still remain there.

@@ -0,0 +1,212 @@
id: 255

This comment has been minimized.

Copy link
@ihsandemir

ihsandemir Sep 25, 2019

Contributor

Should we keep this file at protocol repo as internal hidden?

This comment has been minimized.

Copy link
@mdogan

mdogan Sep 25, 2019

Author Member

@asimarslan requested me to remove it from the official protocol repo. So I decided to keep it here.

This comment has been minimized.

Copy link
@asimarslan

asimarslan Sep 25, 2019

Member

I think we won't need this file anymore. The legacy atomicLong codecs shouldn't be changed and even if they are changed we don't support any kind of backward compatibility.

This comment has been minimized.

Copy link
@mdogan

mdogan Sep 25, 2019

Author Member

I know it's not needed but still I want to keep it as a documentation of codecs.

Additonally removed deprecated IdGenerator.

Moved old AtomicLong impl to an internal package
and renamed it to LongRegister. LongRegister still uses
the old service name to make it backward compatible
while doing perf testing.
LongRegister is using 255 (max id) as codec id.
@mdogan mdogan force-pushed the mdogan:remove-legacy-atomiclong branch from c8a08e1 to 1256258 Sep 26, 2019
@mdogan mdogan merged commit ba4199e into hazelcast:master Sep 26, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@mdogan mdogan deleted the mdogan:remove-legacy-atomiclong branch Sep 26, 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.