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

Benedict commented on CASSANDRA-14821:
--------------------------------------

{quote}The patch itself touches a lot of quite important areas in the code base 
and adds a bunch of test-only code to the production code base
{quote}
Could you elaborate?  I see four category of changes to the main codebase, all 
of which improvements, many of which _nonessential_, so if it's contentious we 
can remove them:
 # MBean registration abstraction, so we can mock it out - this is anyway much 
cleaner than prior copy/paste jobs
 # Internode message parsing - I cleaned up (in my view) some of the netty 
parsing, but we could roll this back. I had already considered deferring it to 
the overall internode messaging review/audit work I'll be undertaking soon. 
Otherwise, this code does not touch the production code paths, it just offers a 
synchronous equivalent for parsing.
 # Executor shutdown - this by definition only happens at shutdown, so is 
essentially a no-op for production running. The reason being that we (lazily) 
depend on daemon thread status to cleanup after ourselves. This is anyway good 
hygiene, and neutral to the codebase.
 # InfiniteLoopExecutor - this is simply a cleanup abstraction of something we 
do in several places, that I suggested we do alongside this, because it made 
sense. It's non-essential, but I don't see the problem with it? It's very 
simple, and improves codebase clarity.

Could you point me to your issues with these, or other issues I missed? I'm 
pretty sure we can assuage your concerns.
{quote}Many people have tried such class-loader approaches in the past and all 
attempts to do so have failed
{quote}
Can you refer me to an example?  This attempt seems pretty damn clean to me, 
and demonstrably works. I've done this kind of thing in the past, and been 
successful.
{quote}How to you intend to test upgrades? 
{quote}
It's not implemented yet, but I've already back ported this to 3.0 successfully 
in order to debug CASSANDRA-14812 (making it *significantly* easier to do so. 
It's surely telling that I would prefer to backport this than debug via CCM?). 
It's a relatively straightforward thing to support multiple versions once the 
core functionality is available in each version we want to test.
{quote}How do you test startup scripts? How do you invoke all the tools? What 
happens if you kill an instance? How to you intend to test upgrades?
{quote}
Almost none of these things are tested (at least in any meaningful sense) in 
today's dtests, I think?

Sure, perhaps deprecating _all_ dtests is maybe over stating it, we'll have to 
wait and see. But the use cases you listed - in the kind of isolated tests 
dtests perform - are just as easily tested by unit tests I think? We're talking 
exclusively about abnormal JVM exit and startup scripts in *extremely curated 
circumstances*. These are local node behaviours, and just as easily tested 
without the distributed aspect, surely? dtests aren't, after all, complex 
randomised tests exploring obscure cluster states.

But if we decide we need a few dtests, sure, I'm not suggesting they'd be 
literally forbidden. I just don't see a strong enough case for them after this 
becomes full-featured and commonplace. I would prefer we ported all dtests to 
this framework.

Lets talk about the advantages of this approach:
 # Dramatically shorter test times
 # Reduced flakiness of tests, as tests can be deterministic instead of based 
on sleeps that are hoped to be sufficient
 # Ability to directly debug failures in your IDE
 # *Vastly* greater control over node behaviours when writing tests
 # Writing tests in the language we write the database in, reducing friction 
costs for most developers on the project
 # Tests can live in-tree, alongside our other tests

So far I've not seen a real downside that you've listed? Why wouldn't we 
deprecate python tests for a majority of use cases?

> Make it possible to run multi-node coordinator/replica tests in a single JVM
> ----------------------------------------------------------------------------
>
>                 Key: CASSANDRA-14821
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14821
>             Project: Cassandra
>          Issue Type: Test
>            Reporter: Alex Petrov
>            Assignee: Alex Petrov
>            Priority: Major
>
> This patch proposes an in-JVM Distributed Tester that can help to write 
> distributed tests in a single JVM and be able to control node behaviour in a 
> fine-grained way and set up nodes exactly how one needs it: configuration 
> settings, parameters, which are also controllable in runtime on a per node 
> basis, so each node can have its own unique state.
> It fires up multiple Cassandra Instances in a single JVM. It is done through 
> having distinct class loaders in order to work around the singleton problem 
> in Cassandra. In order to be able to pass some information between the nodes, 
> a common class loader is used that loads up java standard library and several 
> helper classes. Tests look a lot like CQLTester tests would usually look like.
> Each Cassandra Instance, with its distinct class loader is using 
> serialisation and class loading mechanisms in order to run instance-local 
> queries and execute node state manipulation code, hooks, callbacks etc.
> First version mocks out Messaging Service and simplifies schema management by 
> simply running schema change commands on each of the instances separately. 
> Internode communication is mocked by passing ByteBuffers through shared class 
> loader.
> |[patch|https://github.com/ifesdjeen/cassandra/tree/in-jvm-distributed-tests-2]|[tests|https://circleci.com/workflow-run/d88a1278-596c-4af1-9a03-998e9f6c78d3]|



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to