kevinrr888 opened a new pull request, #5073: URL: https://github.com/apache/accumulo/pull/5073
I did a deep dive into the code executed by `accumulo check-server-config` to figure out what is being checked to see if an instance can be avoided or what could be checked without an instance. Here is root code for reference: https://github.com/apache/accumulo/blob/ed4161ea21dd9b7293be0ca72bc3651d65513440/server/base/src/main/java/org/apache/accumulo/server/conf/CheckServerConfig.java#L30-L34 Here is a summary of my findings: - `SiteConfiguration.auto()` ***does validation***: checks that the deprecated `accumulo-site.xml` file doesn't exist, loads properties from the config file (`accumulo.properties`), checks that both deprecated and the replacement properties aren't both declared in the file, replaces deprecated properties with their non-deprecated versions, and returns a new SiteConfiguration object (the SiteConfiguration object constructor calls `ConfigCheckUtil.validate(config)` to validate the config passed in) - `context.getConfiguration()` ***does validation***. However, as far as I can tell, the only validation done from this call requires an instance (the only validation seems to stem from `ZooBasedConfiguration`/`PropSnapshot`/`PropStore`/`PropStoreKey`/etc.). Validates at least the `instance.secret` property (maybe more). Since this is related to an instance, seems fine that it wouldn't be validated here anyways. - `new ServerContext(SiteConfiguration.auto())` (= `new ServerContext(new ServerInfo(SiteConfiguration.auto()))`) ***does validation***. This creates a `new ServerInfo(siteConfig)` passing that to the `ServerContext` constructor. There is no validation done in the `ServerContext` constructor: most fields are memoized suppliers (the only one we access is `ServerConfigurationFactory serverConfFactory` which is accessed through `context.getConfiguration()` and we went into that code above). The ones that aren't memoized suppliers don't do any validation. But there is validation done from constructing `ServerInfo`. We will take what we can (does some form of validation and is not reliant on an instance) and use that in our new check. With this info, I wrote a new check `CheckServerConfigNoInstance` which, to the best of my knowledge, is all the validation that the current `CheckServerConfig` would be able to do without a running instance. So, this PR adds a new `accumulo check-server-config-no-inst` (in addition to the existing `accumulo check-server-config`) which performs the checks that `accumulo check-server-config` does without the checks that require a running Accumulo instance. Useful for (at least mostly) validating the `accumulo.properties` file without an instance. I'm not sure which branch this should target. Targeting 2.1 for now since the change could be made as early as 2.1 and it may be a desireable command here, but not sure if we would want to add a new command in the 2.1 line. Any input/suggestions on this PR would be appreciated. closes #5034 -- 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]
