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.

Sign up for GitHub

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

Jump to bottom

ISPN-2634 Implement cross-site replication based on IRAC #8244

Merged
merged 1 commit into from May 12, 2020
Merged

ISPN-2634 Implement cross-site replication based on IRAC #8244

merged 1 commit into from May 12, 2020

Conversation

pruivo
Copy link
Member

@pruivo pruivo commented Apr 24, 2020

@pruivo pruivo added the Preview label Apr 24, 2020
@pruivo pruivo force-pushed the t_irac_ng_prv_metadata branch 2 times, most recently from 19ba38c to 70e1955 Compare April 27, 2020 18:07
@pruivo pruivo removed the Preview label Apr 28, 2020
@tristantarrant
Copy link
Member

I would really like to see a server integration test

@pruivo
Copy link
Member Author

pruivo commented May 4, 2020

@tristantarrant added!
Not sure if it is the correct way to do it... but it is working.

@pruivo pruivo force-pushed the t_irac_ng_prv_metadata branch 2 times, most recently from fd84748 to c51b544 Compare May 6, 2020 18:18
wburns
wburns reviewed May 6, 2020
View reviewed changes
Copy link
Member

@wburns wburns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have gotten through just about half, but I figured I should submit before looking again tomorrow.

core/src/main/java/org/infinispan/container/versioning/irac/DefaultIracVersionGenerator.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/container/versioning/irac/IracEntryVersion.java

Merger merger = Merger.NONE;
for (VersionCompare v : vectorClock.values()) {
merger = merger.accept(v);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should short circuit if we ever get back CONFLICT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need. the usual use case is 2 sites and the first compare never generates a conflict.

core/src/main/java/org/infinispan/container/versioning/irac/IracVersionGenerator.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/container/versioning/irac/NoOpIracVersionGenerator.java Show resolved Hide resolved
core/src/main/java/org/infinispan/factories/InterceptorChainFactory.java
@@ -300,6 +319,22 @@ private AsyncInterceptorChain buildInterceptorChain() {
//Nothing...
}

if (cacheMode.isClustered()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (cacheMode.isClustered()) {
if (Configurations.isIracEnabled() && cacheMode.isClustered()) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do that.
Unfortunately, a cache without IRAC enabled can still receive IRAC updates from the remote site and the *IracRemoteInterceptor needs to handle them to discard duplicated or out-of-order updates.

core/src/main/java/org/infinispan/container/versioning/irac/DefaultIracVersionGenerator.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/interceptors/impl/NonTxIracLocalSiteInterceptor.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/xsite/irac/IracManager.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/xsite/irac/DefaultIracManager.java Show resolved Hide resolved
core/src/main/java/org/infinispan/interceptors/xsite/NonTransactionalBackupInterceptor.java Show resolved Hide resolved
ryanemerson
ryanemerson requested changes May 7, 2020
View reviewed changes
core/src/main/java/org/infinispan/metadata/impl/IracMetadata.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/metadata/impl/IracMetadata.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/commands/irac/IracStateResponseCommand.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/commands/irac/IracUpdateKeyCommand.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/commands/irac/IracUpdateKeyCommand.java
public CompletionStage<?> invokeAsync(ComponentRegistry registry) {
//noinspection unchecked
Cache<Object, Object> cache = registry.getCache().running();
//TODO! create a component for BackupReceiver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going into this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely no.
I'm still not sure if I'll create a new component or just merge the code in PerCacheInboundInvocationHandler

In any case, I'll not add more complexity to this PR.

core/src/main/java/org/infinispan/xsite/irac/IracUtils.java Outdated Show resolved Hide resolved
core/src/test/java/org/infinispan/xsite/irac/persistence/BaseIracPersistenceTest.java Show resolved Hide resolved
core/src/test/java/org/infinispan/xsite/irac/persistence/BaseIracPersistenceTest.java Outdated Show resolved Hide resolved
core/src/test/java/org/infinispan/xsite/irac/persistence/BaseIracPersistenceTest.java Outdated

protected abstract void addPersistence(ConfigurationBuilder builder);

protected abstract V wrap(String key, String value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrap and unwrap implementations always return the value except in the case of the IracJpaStoreTest. We should create a default impl of:

   @Override
   protected String wrap(String key, String value) {
      return value;
   }

   @Override
   protected String unwrap(String value) {
      return value;
   }

and override it in IracJpaStoreTest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1
That would require to remove the type V and use casting (and unchecked casts)
I can use org.infinispan.persistence.PersistenceCompatibilityTest.KeyValueWrapper<K,V,T> since I did hit the same "problem" before.

core/src/test/java/org/infinispan/xsite/irac/persistence/BaseIracPersistenceTest.java Outdated Show resolved Hide resolved
@pruivo
Copy link
Member Author

pruivo commented May 8, 2020

updated. I think I handled/replied all your comment @wburns @ryanemerson
thanks for the review!

@pruivo pruivo force-pushed the t_irac_ng_prv_metadata branch 3 times, most recently from c78dc40 to 268dd9b Compare May 11, 2020 15:19
@ryanemerson ryanemerson merged commit 84f3366 into infinispan:master May 12, 2020
@ryanemerson
Copy link
Contributor

Thanks @pruivo

@pruivo pruivo deleted the t_irac_ng_prv_metadata branch May 12, 2020 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wburns wburns wburns left review comments

@ryanemerson ryanemerson ryanemerson requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@pruivo @tristantarrant @ryanemerson @wburns