Skip to content

fixing compatibility issues + testsuite on compatibility#1713

Merged
asfgit merged 2 commits into
apache:masterfrom
clebertsuconic:ARTEMIS-1546
Dec 19, 2017
Merged

fixing compatibility issues + testsuite on compatibility#1713
asfgit merged 2 commits into
apache:masterfrom
clebertsuconic:ARTEMIS-1546

Conversation

@clebertsuconic
Copy link
Copy Markdown
Contributor

No description provided.

@clebertsuconic clebertsuconic changed the title Adding compatibility issues + testsuite on compatibility fixing compatibility issues + testsuite on compatibility Dec 14, 2017
@michaelandrepearce
Copy link
Copy Markdown
Contributor

michaelandrepearce commented Dec 14, 2017

@clebertsuconic instead of this mass interface changes to pass around CoreRemotingConnection, why not add ability to simply set the packet (either using default method and field java 8, or could be done in PacketImpl) with the channel (connection) version as it arrives before it gets decoded and passed through channel handlers, and then the information will be with the packet, where it needs special handling, would avoid this larger interface change.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@michaelandrepearce I thought it would be more complete that way.. would make sure we would have the connection during the encoding for anything else we needed.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@mtaylor I added hornetq to the picture here now. and it looks good.

next step is to add 1.4 as server.. and I see a few issues now.

@mtaylor
Copy link
Copy Markdown
Contributor

mtaylor commented Dec 15, 2017

@clebertsuconic @michaelandrepearce We actually already have ClientPacketDecoder and ServerPacketDecoder which are supposed to be used to encapsulate the decoding logic. Why not just implement a new ServerPacketDecoder1_4. This gets set up on connection entry so you have the version info already. We don't use an encoder on the way out, but we could easily add something similar?

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@mtaylor the parsing of the Message happens inside the CoreMessage.. trying to avoid copy between the incoming buffer...

Move "the hack" to PacketImpl would be a bit complex...

But i will minimize the number of changes.

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-1546 branch 3 times, most recently from 6f2b340 to 030403f Compare December 15, 2017 22:17
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@mtaylor the "hack" still in message but it's activated through the Packets... I still had to add connection to a lot of places.. as well as making the version-id orthogonal..

This is also fixing 1.4 as server as I added a new change.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@michaelandrepearce I tried to simplify as much as I could

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-1546 branch 3 times, most recently from ae65176 to dc93ed3 Compare December 16, 2017 01:47
// }
if (bindings == null && context.getAddress() != null) {
AddressSettings settings = server.getAddressSettingsRepository().getMatch(context.getAddress().toString());
if (settings.isAutoCreateQueues()) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mtaylor Please review this.. this is fixing the compatibility between a 1.4 server and a 2.5 (snapshot) client.

CreateAddressMessage request = new CreateAddressMessage(address, routingTypes, autoCreated, true);
sessionChannel.sendBlocking(request, PacketImpl.NULL_RESPONSE);
if (!sessionChannel.getConnection().isVersionBeforeAddressChange()) {
sessionChannel.sendBlocking(request, PacketImpl.NULL_RESPONSE);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mtaylor this is fixing some compatibilty issues

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

@mtaylor with this last commit, this completes the full mesh. Everything compatible.. at least on these tests I have now.. I'm adding more next week.

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-1546 branch 4 times, most recently from dfe419c to f5bf9d0 Compare December 16, 2017 03:26
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

DON'T MERGE THIS...

@mtaylor help me review the auto-create stuff on that side... perhaps we only do that for previous versions...

Testsuite is broken after this last commit..I can take out some of the auto-create, but we still need the other stuff...

lets talk on monday...

So... again... DON'T MERGE THIS yet

@clebertsuconic clebertsuconic changed the title fixing compatibility issues + testsuite on compatibility WIP-DO-NOT-MERGE fixing compatibility issues + testsuite on compatibility Dec 16, 2017
@clebertsuconic clebertsuconic force-pushed the ARTEMIS-1546 branch 2 times, most recently from 80ac192 to 23282db Compare December 19, 2017 02:56
https://issues.apache.org/jira/browse/ARTEMIS-1546

- the dependency scan is changed to allow adding an extra repository
- adding groovy so we won't require compilation dependencies (just runtime)
  without needing reflection (thanks Groovy :) )
- Adding hornetq to the mesh of version tests
https://issues.apache.org/jira/browse/ARTEMIS-1546

Recasting the body as 1.x format when there's a 1.x in use at the other size of the wire
@clebertsuconic clebertsuconic changed the title WIP-DO-NOT-MERGE fixing compatibility issues + testsuite on compatibility fixing compatibility issues + testsuite on compatibility Dec 19, 2017
@asfgit asfgit merged commit dbe575a into apache:master Dec 19, 2017
asfgit pushed a commit that referenced this pull request Dec 19, 2017
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.

4 participants