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-12177 Persist IRAC version during shutdown #8680

Merged
merged 1 commit into from Oct 23, 2020
Merged

ISPN-12177 Persist IRAC version during shutdown #8680

merged 1 commit into from Oct 23, 2020

Conversation

pruivo
Copy link
Member

@pruivo pruivo commented Sep 10, 2020

@tristantarrant
Copy link
Member

Conflicts

@pruivo pruivo force-pushed the t_12177_irac_version_persist branch from e204caf to 4539874 Compare October 9, 2020 09:46
wburns
wburns requested changes Oct 22, 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.

So I see where it loads from the file, but isn't there supposed to be something to read the state from the other site? Or is that handled elsewhere?

And what about if we have data in a store, guessing that is a different PR?

core/src/main/java/org/infinispan/commands/irac/IracMetadataRequestCommand.java
@@ -90,6 +92,7 @@ public String toString() {
"cacheName=" + cacheName +
", segment=" + segment +
", topologyId=" + topologyId +
Copy link
Member

Choose a reason for hiding this comment

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

Looks like topologyId can be removed as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

not really. It makes sure the node has at least that topology installed before handling the executing the command.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be part of the readFrom and writeTo though?

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, the previous code was incorrect.
The CacheRpcCommandExternalizer [1] invokes the ReplicableCommandExternalizer [2] and the latter sends the topology id.

[1] https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/marshall/exts/CacheRpcCommandExternalizer.java#L142
[2] https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/marshall/exts/ReplicableCommandExternalizer.java#L82

core/src/main/java/org/infinispan/container/versioning/irac/TopologyIracVersion.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/container/versioning/irac/TopologyIracVersion.java Outdated Show resolved Hide resolved
core/src/main/java/org/infinispan/interceptors/impl/NonTxIracLocalSiteInterceptor.java Show resolved Hide resolved
@pruivo
Copy link
Member Author

pruivo commented Oct 22, 2020 •
edited

Each node, when a rebalance happends, broadcast its version state to all other nodes (see org.infinispan.container.versioning.irac.DefaultIracVersionGenerator#onTopologyChange())
edit: https://github.com/infinispan/infinispan/pull/8680/files#diff-95f7be6255d4ac1c28c4294aa8e1dacd3b38b303c0c51fd85b2bb32c98465a46R103

@pruivo
Copy link
Member Author

pruivo commented Oct 22, 2020

About the cache store, it uses the version in the context to generate a new higher version. Example, for non-tx caches: https://github.com/infinispan/infinispan/pull/8680/files#diff-8f85a96763cdc57d2a36e8a8b873178ab509a3083766c6d2ff9bb56a07a1a2c6R205

this covers "all" scenarios except if you use SKIP_CACHE_LOAD + !preload + !conditional_write

@pruivo pruivo force-pushed the t_12177_irac_version_persist branch from 4539874 to d3c583b Compare October 22, 2020 18:03
@pruivo
Copy link
Member Author

pruivo commented Oct 22, 2020

updated & rebased

@pruivo pruivo force-pushed the t_12177_irac_version_persist branch from d3c583b to 50a3e1c Compare October 23, 2020 10:40
@pruivo
Copy link
Member Author

pruivo commented Oct 23, 2020

fixed test failure

@wburns wburns merged commit 971632d into infinispan:master Oct 23, 2020
@wburns
Copy link
Member

wburns commented Oct 23, 2020

Integrated into master, thanks @pruivo !

@pruivo
Copy link
Member Author

pruivo commented Oct 23, 2020

yey! thanks @wburns !

@pruivo pruivo deleted the t_12177_irac_version_persist branch October 23, 2020 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wburns wburns wburns requested changes

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