ctubbsii commented on code in PR #5073:
URL: https://github.com/apache/accumulo/pull/5073#discussion_r1850975374
##########
server/base/src/main/java/org/apache/accumulo/server/conf/CheckServerConfig.java:
##########
@@ -18,17 +18,45 @@
*/
package org.apache.accumulo.server.conf;
+import java.io.IOException;
+
import org.apache.accumulo.core.conf.SiteConfiguration;
import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.ServerDirs;
+import org.apache.accumulo.server.fs.VolumeManager;
+import org.apache.accumulo.server.fs.VolumeManagerImpl;
import org.apache.accumulo.start.spi.KeywordExecutable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import com.google.auto.service.AutoService;
@AutoService(KeywordExecutable.class)
public class CheckServerConfig implements KeywordExecutable {
public static void main(String[] args) {
- try (var context = new ServerContext(SiteConfiguration.auto())) {
+ var siteConfig = SiteConfiguration.auto();
+ var hadoopConfig = new Configuration();
+ VolumeManager volumeManager;
+ try {
+ volumeManager = VolumeManagerImpl.get(siteConfig, hadoopConfig);
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ }
+ var serverDirs = new ServerDirs(siteConfig, hadoopConfig);
+ Path instanceIdPath =
serverDirs.getInstanceIdLocation(volumeManager.getFirst());
+ // Check if an instance exists. If it doesn't, we have performed all the
checks we can.
+ // If it does, we can perform further checks.
+ try {
+ VolumeManager.getInstanceIDFromHdfs(instanceIdPath, hadoopConfig);
+ } catch (Exception e) {
+ System.out.println("WARNING : Performed only a subset of checks (which
passed). There is a "
Review Comment:
I'm not really sure. It seems to me that the original intent of the check,
like most checks, are to check *this* instance. I'm not sure that this command
makes any sense at all to run without an instance, so I think just having an
error is fine in those circumstances.
It seems like the use case that #5034 was trying to report was different
than this check was written for. That use case seems to be "I want to validate
this config file, independently of any Accumulo instance". For that case, I
think a much simpler solution can be done... just provide a config file as an
optional argument, do `SiteConfiguration.fromFile(new File(path)).build()`
using the specified file to validate it, rather than using
`SiteConfiguration.auto()`, and then skip any instance-specific checks after
that. After all, that use case is just asking for validation of the contents of
a config file, not validation of any particular instance's configuration.
--
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]
