[
https://issues.apache.org/jira/browse/HDFS-14567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16910096#comment-16910096
]
Wei-Chiu Chuang commented on HDFS-14567:
----------------------------------------
Reviewed the test code.
I am okay with the test in general.
* It would be great to not use Thread.sleep() to control thread order and use
FakeTimer instead. If you really don't want to use FakeTimer because it is
potentially a bigger change, please use a larger sleep time. Say 1 second.
Because the Jenkins is very busy and being too precise does no good to you.
* You don't need to instantiate MiniKMS at all. You just test the behavior of
KMSAcl. Remove it reduces the test run time significantly from 7+ seconds to
less than 1 second.
* Please don't swallow exceptions (catch Exception and do nothing). By swallow
exceptions you don't know if the test passes or fails. Actually it looks to me
the test if flaky after stopping swapping exceptions.
> If kms-acls is failed to load, and it will never be reload
> -----------------------------------------------------------
>
> Key: HDFS-14567
> URL: https://issues.apache.org/jira/browse/HDFS-14567
> Project: Hadoop HDFS
> Issue Type: Bug
> Reporter: hemanthboyina
> Assignee: hemanthboyina
> Priority: Major
> Attachments: HDFS-14567.patch
>
>
> Scenario : through one automation tool , we are generating kms-acls , though
> the generation of kms-acls is not completed , the system will detect a
> modification of kms-alcs and it will try to load
> Before getting the configuration we are modifiying last reload time , code
> shown below
> {code:java}
> private Configuration loadACLsFromFile() {
> LOG.debug("Loading ACLs file");
> lastReload = System.currentTimeMillis();
> Configuration conf = KMSConfiguration.getACLsConf();
> // triggering the resource loading.
> conf.get(Type.CREATE.getAclConfigKey());
> return conf;
> }{code}
> if the kms-acls file written within next 100ms , the changes will not be
> loaded as this condition "newer = f.lastModified() - time > 100" never meets
> because we have modified last reload time before getting the configuration
> {code:java}
> public static boolean isACLsFileNewer(long time) {
> boolean newer = false;
> String confDir = System.getProperty(KMS_CONFIG_DIR);
> if (confDir != null) {
> Path confPath = new Path(confDir);
> if (!confPath.isUriPathAbsolute()) {
> throw new RuntimeException("System property '" + KMS_CONFIG_DIR +
> "' must be an absolute path: " + confDir);
> }
> File f = new File(confDir, KMS_ACLS_XML);
> LOG.trace("Checking file {}, modification time is {}, last reload time is"
> + " {}", f.getPath(), f.lastModified(), time);
> // at least 100ms newer than time, we do this to ensure the file
> // has been properly closed/flushed
> newer = f.lastModified() - time > 100;
> }
> return newer;
> } {code}
>
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
