Skip to content

ARTEMIS-3767 Fix replication incompatibility between pre 2.18.0 and SNAPSHOT (WIP)#4144

Closed
jsmucr wants to merge 9 commits intoapache:mainfrom
jsmucr:ARTEMIS-3767
Closed

ARTEMIS-3767 Fix replication incompatibility between pre 2.18.0 and SNAPSHOT (WIP)#4144
jsmucr wants to merge 9 commits intoapache:mainfrom
jsmucr:ARTEMIS-3767

Conversation

@jsmucr
Copy link
Contributor

@jsmucr jsmucr commented Jul 12, 2022

This PR attempts to solve the issue described in https://issues.apache.org/jira/browse/ARTEMIS-3767.

TL;DR replication between =<2.17.0 and newer Artemis versions is broken since 2.18.0.

jsmucr added 9 commits July 12, 2022 14:58
The only test currently present focuses on replication issues introduced in 2.18.0 (ARTEMIS-3767).
This test suite has been inspired by compatibility-tests, but it's been generalized in a way that individual servers can be tested against each other too. All contexts are now fully separated without binding and classpath leaks present in the original suite.
@jsmucr
Copy link
Contributor Author

jsmucr commented Jul 14, 2022

I've got a problem with this. Changes in #3680 should have introduced a new version of the REPLICATION_START_FINISH_SYNC packet as there's suddenly more data in it. This tainted the Artemis releases up to the current snapshot and I don't know how to overcome this unless there's a bugfix release for all recent versions starting with 2.18.0.

@clebertsuconic
Copy link
Contributor

I don't think you need the Server compatibility Test.

Please take a look at MultiVersionReplicatest under compatibility-tests

@clebertsuconic clebertsuconic marked this pull request as draft July 15, 2022 15:31
@jsmucr
Copy link
Contributor Author

jsmucr commented Jul 15, 2022

I didn't notice this test before. Maybe it wasn't on the 2.18.0 I was starting with, and after many failures when trying to make it work with the compatibility-tests base, I created my own (more Groovy-ish way, without shared bindings and the GroovyRun helper class). Well, nevermind. I'll throw it away then.
The test does not work with 2.17.0 now as it appears to assume the existence of some newer methods on older implementations, but that's easy to fix, I suppose.

@gemmellr
Copy link
Member

@clebertsuconic
Copy link
Contributor

lets move the discussion to my new PR?

this PR is being replaced by #4150

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants