adoroszlai commented on code in PR #7652:
URL: https://github.com/apache/ozone/pull/7652#discussion_r1905354533
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBDefinitionFactory.java:
##########
@@ -98,6 +99,36 @@ public static DBDefinition getDefinition(Path dbPath,
return getDefinition(dbName);
}
+ public static DBDefinition getDefinition(Path dbPath, ConfigurationSource
config, String overrideDBDef) {
+ if (overrideDBDef == null) {
+ return getDefinition(dbPath, config);
+ }
+ try {
+ Class<?> clazz = Class.forName(overrideDBDef);
+ if (DBDefinition.class.isAssignableFrom(clazz)) {
+ DBDefinition instance;
+ try {
+ Constructor<?> noParamConstructor = clazz.getDeclaredConstructor();
+ noParamConstructor.setAccessible(true);
+ instance = (DBDefinition) noParamConstructor.newInstance();
+ } catch (NoSuchMethodException e) {
+ Constructor<?> stringParamConstructor =
clazz.getDeclaredConstructor(String.class);
+ stringParamConstructor.setAccessible(true);
+ instance = (DBDefinition)
stringParamConstructor.newInstance(dbPath.toAbsolutePath().toString());
+ }
+ return instance;
Review Comment:
Some DB definitions are implemented as singletons, we can use the factory
method `get()`.
Example:
```java
Method factory = clazz.getDeclaredMethod("get");
instance = (DBDefinition) factory.invoke(clazz);
```
I suggest also trying constructor that accepts `config` instead of adding
"unused" constructors.
Since now we would have 4 different ways to get an instance, let's define
those ways as functions:
```java
private static List<CheckedSupplier<DBDefinition, Exception>> getFactories(
Class<?> clazz, String dbPathString, ConfigurationSource config) {
return Arrays.asList(
() -> {
Method factoryMethod = clazz.getDeclaredMethod("get");
factoryMethod.setAccessible(true);
return (DBDefinition) factoryMethod.invoke(clazz);
},
() -> {
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
return (DBDefinition) constructor.newInstance();
},
() -> {
Constructor<?> constructor =
clazz.getDeclaredConstructor(String.class);
constructor.setAccessible(true);
return (DBDefinition) constructor.newInstance(dbPathString);
},
() -> {
Constructor<?> constructor =
clazz.getDeclaredConstructor(String.class, ConfigurationSource.class);
constructor.setAccessible(true);
return (DBDefinition) constructor.newInstance(dbPathString,
config);
}
);
}
```
and loop over their list:
```java
String dbPathString = dbPath.toAbsolutePath().toString();
for (CheckedSupplier<DBDefinition, Exception> factory :
getFactories(clazz, dbPathString, config)) {
try {
return factory.get();
} catch (Exception ignored) {
}
}
throw new IllegalArgumentException("Could not get instance of " +
overrideDBDef);
```
Please feel free to tweak as needed.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBDefinitionFactory.java:
##########
@@ -98,6 +99,36 @@ public static DBDefinition getDefinition(Path dbPath,
return getDefinition(dbName);
}
+ public static DBDefinition getDefinition(Path dbPath, ConfigurationSource
config, String overrideDBDef) {
+ if (overrideDBDef == null) {
+ return getDefinition(dbPath, config);
+ }
+ try {
+ Class<?> clazz = Class.forName(overrideDBDef);
+ if (DBDefinition.class.isAssignableFrom(clazz)) {
+ DBDefinition instance;
+ try {
+ Constructor<?> noParamConstructor = clazz.getDeclaredConstructor();
+ noParamConstructor.setAccessible(true);
+ instance = (DBDefinition) noParamConstructor.newInstance();
+ } catch (NoSuchMethodException e) {
+ Constructor<?> stringParamConstructor =
clazz.getDeclaredConstructor(String.class);
+ stringParamConstructor.setAccessible(true);
+ instance = (DBDefinition)
stringParamConstructor.newInstance(dbPath.toAbsolutePath().toString());
+ }
+ return instance;
+ } else {
+ System.err.println("Class does not implement DBDefinition: " +
overrideDBDef);
+ }
+ } catch (ClassNotFoundException e) {
+ System.err.println("Class not found: " + overrideDBDef);
+ } catch (InstantiationException | IllegalAccessException |
NoSuchMethodException |
+ SecurityException | java.lang.reflect.InvocationTargetException
e) {
+ System.err.println("Error creating instance: " + e.getMessage());
Review Comment:
I think we should throw exception here. Simply returning `null` will cause
additional error message to be displayed:
```
Error: Incorrect DB Path
```
--
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]
