dcapwell commented on code in PR #3836:
URL: https://github.com/apache/cassandra/pull/3836#discussion_r1937754037
##########
test/distributed/org/apache/cassandra/distributed/impl/AbstractCluster.java:
##########
@@ -332,29 +333,12 @@ private IInvokableInstance newInstance()
++generation;
IClassTransformer transformer = classTransformer == null ? null :
classTransformer.initialise();
ClassLoader classLoader = new InstanceClassLoader(generation,
config.num(), version.classpath, sharedClassLoader, sharedClassPredicate,
transformer);
- Consumer<Throwable> stabilityInspector;
- {
- try
- {
- Class<?> owner =
classLoader.loadClass(JVMStabilityInspector.class.getName());
- Method method = owner.getMethod("inspectThrowable",
Throwable.class);
Review Comment:
the main reason is that this logic is trying to pull `JVMStabilityInspector`
into the JVM-Dtest api which makes things harder for upgrade testing... now any
change to that class must be coordinated cross every branch, and you can no
longer test *released* versions...
I don't see a clear reason to bring this internal API into the public api
when we already have `instance.uncaughtException` that solves this issue for us.
> could you elaborate?
`uncaughtException` comes with extra logging so when you look at the logs
you see these happening, when you call `inspectThrowable` those logs don't
happen which makes debugging a bit harder. I know that an error happened, but
"where" did it happen? 🤷
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
