ctubbsii commented on a change in pull request #300: ACCUMULO-4708 Limit RFile
block size to 2GB
URL: https://github.com/apache/accumulo/pull/300#discussion_r140381442
##########
File path:
core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
##########
@@ -51,4 +51,11 @@ public void testGetProperties() {
public void testSanityCheck() {
ConfigSanityCheck.validate(c);
}
+
+ @Test
+ public void testDefaultBlockSizeIsAllowed() {
+ long bsi = c.getAsBytes(Property.TABLE_FILE_COMPRESSED_BLOCK_SIZE_INDEX);
+ assert (bsi > 0);
+ assert (bsi < Integer.MAX_VALUE);
Review comment:
Probably best to avoid assert keyword statements, because assertions can be
disabled in the JVM. Best to use JUnit provided APIs for assertions:
`Assert.assertTrue( expr )`
Also, might be good to add a small one-line comment explaining what we're
checking for. As I understand it, this is mostly just a sanity check on our
default.
We can check more generally that any value in the config is suitable by
putting this check in:
https://github.com/apache/accumulo/blob/master/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java#L46
in the `validate()` method. This gets executed whenever we read various
configurations. We could add a check to ensure that whatever the user has
configured, whichever configuration they've configured it in, that it has a
reasonable value. This is very similar to how we're already checking that the
INSTANCE_ZK_TIMEOUT value is within an allowable range.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
