adoroszlai commented on code in PR #6155:
URL: https://github.com/apache/ozone/pull/6155#discussion_r1477011816
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -125,6 +125,9 @@ public class HddsDatanodeService extends GenericCli
implements ServicePlugin {
private HddsDatanodeClientProtocolServer clientProtocolServer;
private OzoneAdmins admins;
private ReconfigurationHandler reconfigurationHandler;
+ // Default datanode initial and current version to be used when datanode.id
file doesn't exist
+ // Note: Currently this is only used in tests.
+ private int defaultInitialVersion = -1, defaultCurrentVersion = -1;
Review Comment:
I don't think we should add these (and >= 0 logic + the get/set methods) to
enable testing. Change this class to access `DatanodeVersion.CURRENT_VERSION`
via a static method, and then mock that method in tests where needed.
https://javadoc.io/doc/org.mockito/mockito-core/4.11.0/org/mockito/Mockito.html#48
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java:
##########
@@ -138,23 +138,32 @@ public class MiniOzoneClusterImpl implements
MiniOzoneCluster {
private final Set<AutoCloseable> clients = ConcurrentHashMap.newKeySet();
private SecretKeyClient secretKeyClient;
+ // TODO: Get rid of Optional, just use null
+ private Optional<int[]> dnInitialVersion = Optional.empty();
+ private Optional<int[]> dnCurrentVersion = Optional.empty();
+
/**
* Creates a new MiniOzoneCluster with Recon.
*
* @throws IOException if there is an I/O error
*/
+ @SuppressWarnings("checkstyle:ParameterNumber")
MiniOzoneClusterImpl(OzoneConfiguration conf,
SCMConfigurator scmConfigurator,
OzoneManager ozoneManager,
StorageContainerManager scm,
List<HddsDatanodeService> hddsDatanodes,
- ReconServer reconServer) {
+ ReconServer reconServer,
+ Optional<int[]> dnInitialVersion,
+ Optional<int[]> dnCurrentVersion) {
Review Comment:
The two arrays passed here is not at the same level of abstraction as the
other parameters. Handling initial and current versions would be better done
when creating the `HddsDatanodeService` instances. That way we could avoid
cluttering the `MiniOzoneCluster` interface.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
