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

ISPN-12646 IllegalArgumentException when doing Rolling Upgrades on transactional caches #9033

Merged

Conversation

gustavocoding
Copy link

@gustavocoding
Copy link
Author

Failures unrelated

@tristantarrant
Copy link
Member

Merged from other PR

@gustavocoding
Copy link
Author

@tristantarrant The other PR didn't have all the commits from this one, I will rebase

@tristantarrant
Copy link
Member

oops

@gustavocoding gustavocoding force-pushed the ISPN-12646_rolling_upgrade_tx_master branch from 80393ac to 809f2ff Compare March 10, 2021 17:16
@gustavocoding
Copy link
Author

should be ready to merge @tristantarrant

Integer DISCONNECT_REMOTE_STORE = 1902;

Integer ENTRY_WRITER = 1902;
Integer DISCONNECT_REMOTE_STORE = 1903;
Copy link
Member

Choose a reason for hiding this comment

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

why increasing the DISCONNECT_REMOTE_STORE id? couldn't ENTRY_WRITER be 1093?

Copy link
Author

Choose a reason for hiding this comment

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

No reason, it was a merge issue, let me put it back

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@gustavocoding gustavocoding force-pushed the ISPN-12646_rolling_upgrade_tx_master branch from 809f2ff to 140929d Compare March 12, 2021 10:47
@@ -102,6 +103,9 @@ private boolean skewed(IncrementableEntryVersion prevVersion, EntryVersion versi
}
IncrementableEntryVersion prevVersion = versionFromEntry(ice);
if (prevVersion == null) {
if (rollingUpgrade) {
return versionGenerator.nonExistingVersion();
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to return nonExistingVersion() every time, without adding the rollingUpgrade parameter?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, wdyt @pruivo ?

Copy link
Member

Choose a reason for hiding this comment

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

we never have prevVersion==null except for the rolling upgrade scenario.
Hot Rod isn't aware of the internal version and Infinispan stores the entry from RemoteStore as is.
I would leave the exception there at least to make sure there are no errors.

@pruivo pruivo merged commit 8736acf into infinispan:master Mar 12, 2021
@pruivo
Copy link
Member

pruivo commented Mar 12, 2021

integrated! thanks @gustavonalle !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants