[jira] [Comment Edited] (CASSANDRA-14937) Multi-version In-JVM dtests

["Benedict (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-14937?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16746451#comment-16746451 ] 

Benedict edited comment on CASSANDRA-14937 at 1/18/19 4:27 PM:
---------------------------------------------------------------

Thanks for the quick review! Before I had even had a chance to post a comment explaining the latest changes.

These are mostly excellent points, but a few quibbles:
{quote}it seems that IMessage doesn't have to be interface with each version having it's own implementation
{quote}
This is necessary to support cross-version compatibility. There needs to be a common parent that is shared across the class loaders, so that older versions can access the contents of the newer messages (and vice versa)
{quote}new Object[][] here can be constant
{quote}
I deliberately chose not to do that here, since this is an uncommon code path, it's test code, and it's unnecessary boilerplate. I don't mind changing if you have a strong opinion on it though.
{quote}probably this will be documented, but "build" directory is currently assumed to be under resources or?
{quote}
The build directory is in the project root, i.e. where {{ant dtest-jar}} writes it. Probably this needs a wiki page or something documenting it, besides some improved javadoc (that is in progress)
{quote}if you at 3.0 branch, you can't start 3.11 or 4.0 cluster because of CDC. On the more general note. Since configs are generated from "cluster" perspective (in AbstractCluster#create, we do not get to pick a right config for the version.
{quote}
It's only intended to be backwards compatible, not forwards. Possibly you were mislead by the UpgradeTest which may have been backported late last night after extensive refactoring to the patch yesterday - I haven't yet cleaned up the final touches of the various ports.

There's also backwards compatibility translations baked into the InstanceConfig; for most fields this is managed by simply ignoring missing properties when populating the config, and some specific legacy properties being specified in all future versions as well.  For 4.0 there is also special version-aware logic to strip the seed provider list of its ports.
{quote}InstanceClassLoader#id is now unused
{quote}
As discussed on a previous JIRA, the idea of this was to aid in debugging. But now we annotate the threads with the information, it's probably unnecessary, so will remove it.
{quote}it looks like right now, to test current version
{quote}
No, you can already use the Versions.CURRENT public static final field, as is used for the regular DistributedReadWritePathTest, but it's a TODO to automatically include that in the Versions.find() (which currently only looks for jars). Thanks for reminding me.
{quote}it looks like we can not create mixed-version cluster at the moment even though nothing prevents it. Maybe it was planned for future? For example, create a cluster with 3.0 and 4.0 without upgrades. I see quite a few use-cases for that.
{quote}
I originally had planned to support that, but rolled back the decision since it logically didn't seem to make much sense. We definitely don't support starting a brand new cluster with a mixed version? We can reintroduce it if we want, but it's fairly straight forward to stop/start the node with a new version too.

I'll address the other items you point out shortly.


was (Author: benedict):
Thanks for the quick review! Before I had even had a chance to post a comment explaining the latest changes.

These are mostly excellent points, but a few quibbles:
{quote}it seems that IMessage doesn't have to be interface with each version having it's own implementation
{quote}
This is necessary to support cross-version compatibility. There needs to be a common parent that is shared across the class loaders, so that older versions can access the contents of the newer messages (and vice versa)
{quote}new Object[][] here can be constant
{quote}
I deliberately chose not to do that here, since this is an uncommon code path, it's test code, and it's unnecessary boilerplate. I don't mind changing if you have a strong opinion on it though.
{quote}probably this will be documented, but "build" directory is currently assumed to be under resources or?
{quote}
The build directory is in the project root, i.e. where {{ant dtest-jar}} writes it. Probably this needs a wiki page or something documenting it, besides some improved javadoc (that is in progress)
{quote}if you at 3.0 branch, you can't start 3.11 or 4.0 cluster because of CDC. On the more general note. Since configs are generated from "cluster" perspective (in AbstractCluster#create, we do not get to pick a right config for the version.
{quote}
It's only intended to be backwards compatible, not forwards. Possibly you were mislead by the UpgradeTest which may have been backported late last night after extensive refactoring to the patch yesterday - I haven't yet cleaned up the final touches of the various ports.
{quote}InstanceClassLoader#id is now unused
{quote}
As discussed on a previous JIRA, the idea of this was to aid in debugging. But now we annotate the threads with the information, it's probably unnecessary, so will remove it.
{quote}it looks like right now, to test current version
{quote}
No, you can already use the Versions.CURRENT public static final field, as is used for the regular DistributedReadWritePathTest, but it's a TODO to automatically include that in the Versions.find() (which currently only looks for jars). Thanks for reminding me.
{quote}it looks like we can not create mixed-version cluster at the moment even though nothing prevents it. Maybe it was planned for future? For example, create a cluster with 3.0 and 4.0 without upgrades. I see quite a few use-cases for that.
{quote}
I originally had planned to support that, but rolled back the decision since it logically didn't seem to make much sense. We definitely don't support starting a brand new cluster with a mixed version? We can reintroduce it if we want, but it's fairly straight forward to stop/start the node with a new version too.

I'll address the other items you point out shortly.

> Multi-version In-JVM dtests
> ---------------------------
>
>                 Key: CASSANDRA-14937
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14937
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Test/dtest
>            Reporter: Benedict
>            Assignee: Benedict
>            Priority: Major
>             Fix For: 2.2.x, 3.0.x, 3.11.x
>
>
> In order to support more sophisticated upgrade tests, including complex fuzz tests that can span a sequence of version upgrades, we propose abstracting a cross-version API for the in-jvm dtests.  This will permit starting a node with an arbitrary compatible C* version, stopping the node, and restarting it with another C* version.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org