Add integration test that ensures that server process does not start against newer version of accumulo. #4513
Conversation
|
MiniAccumuloClusterImpl.verifyUp (called from start) should throw an IllegalStateException if the expected processes are not running. |
test/src/main/java/org/apache/accumulo/test/ServerVersionIT.java
Outdated
Show resolved
Hide resolved
test/src/main/java/org/apache/accumulo/test/ServerVersionIT.java
Outdated
Show resolved
Hide resolved
test/src/main/java/org/apache/accumulo/test/ServerVersionIT.java
Outdated
Show resolved
Hide resolved
| ExecutorService executor = Executors.newSingleThreadExecutor(); | ||
| Future<?> future = executor.submit(() -> { | ||
| for (ServerType st : ServerType.values()) { | ||
| try { | ||
| getCluster().getClusterControl().start(st); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| }); | ||
| // Check to see if servers fail to start | ||
| assertThrows(TimeoutException.class, () -> future.get(20, TimeUnit.SECONDS)); |
There was a problem hiding this comment.
I think this will only check that the first ServerType in ServerType.values() will timeout. We might want to create a future for each ServerType and check that each one times out. That will make this test pretty long but will make sure that none of them start up.
There was a problem hiding this comment.
I looked into starting each server individually and it like only couple servers actually time out, the rest seem to start normally. For the check is it worth checking each servers behavior or just look for the ones that timeout?
There was a problem hiding this comment.
We probably want to ensure that all server types don't load with a mismatched data version.
If they are starting normally then that could cause issues.
|
A more descriptive name for this IT might be better. |
|
Since ITs don't run in GitHub Actions by default, I kicked off a special build at https://github.com/apache/accumulo/actions/runs/10293114308/job/28488804041 to run it. |
|
|
Okay that newer job definitely passed. |
| for (ServerType st : ServerType.values()) { | ||
|
|
||
| if (!st.getDeclaringClass().isAnnotationPresent(Deprecated.class)) { | ||
| Process p = getCluster().exec(Main.class, st.prettyPrint()).getProcess(); |
There was a problem hiding this comment.
This doesn't seem to be working as intended.
I ran the IT locally and looked at the log files for the Main class.
accumulo.test.DataVersionIT_SharedMiniClusterBase/lib/ext
Invalid argument: Java <main class> 'TServer' was not found. Please use the wholly qualified package name.
So the test is passing because all processes are failing to start. However, it's not because of the VersionNumber mismatch, it's just due to an invalid class name being used.
Looking at a running TServer process, the process is started with the full class name: org.apache.accumulo.start.Main org.apache.accumulo.tserver.TabletServer
|
|
||
| assertTrue(fs.exists(rootPath)); | ||
|
|
||
| getCluster().stop(); |
There was a problem hiding this comment.
This will probably stop the zookeeper nodes as well.
If servers can't talk to ZK they will fail to start.
You probably want to turn ZK back on.
| getCluster().stop(); | |
| getCluster().stop(); | |
| getCluster().getClusterControl().start(ServerType.ZOOKEEPER); |
At the current moment, checking for errors while each server type starts up fails.
We should be expecting them to throw errors during start up since it should detect that there is a different version of hdfs, but at the moment they are not giving errors.
issue #4268