[ https://issues.apache.org/jira/browse/YARN-7497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16418354#comment-16418354 ]
Jiandan Yang commented on YARN-7497: ------------------------------------- Hi, [~cheersyang]. Thanks very much for so quickly response and very detailed advice. {noformat} My major concern over the patch is that it doesn't use WAL fashion to prevent potential inconsistent in-memory/persisted conf. I suggest to implement the log/retrieve logic to make sure it works that way. Something like ZKConfigurationStore. {noformat} As we discussed offline, all configuration items are written to the file system every time it is persisted, so there is no need to keep the incremental log. And in this version of the patch, in the method *logMutation* writing temp configuration file persistence, and in method *confirmMutation*, if isValid is true, finalize temp configuration file, otherwise delete the temp configuration file, and rollback *schedConf* {noformat} I feel it is better to call FSSchedulerConfigurationStore }}instead of {{HDFSSchedulerConfigurationStore as this is should work as long as on any HFS. {noformat} I have renamed to FSSchedulerConfigurationStore. {noformat} *Conf* Need to add some doc in yarn-site.xml, property yarn.scheduler.configuration.store.class; also the added configuration files, make sure TestYarnConfiguration passes. {noformat} added configurations are in FSSchedulerConfigurationStore. it does not make TestYarnConfiguration failed, I will move config to YarnConfiguration later. {noformat} *TestMutableCSConfigurationProvider* writeConf: please use try-finally to make sure both fileSystem and outputStream are closed {noformat} My fault,and fix it by try-with-resources statement {noformat} HDFSSchedulerConfigurationStore line 52: typo HFDS line 63, 74: pendingId not used line 68: typo configFilePathFileter -> configFilePathFilter line 78: better to add a null check before using path line 86: I don't think we need to throw a RuntimeException here, can we replace it with IOException? So CapacityScheduler can handle this during initScheduler line 102, 145: configInputStream is not closed line 205: move outputStream to finally statement {noformat} I have update code according your opinion except *line 102, 145: configInputStream is not closed*, because Configuration#addResource will close InputStream after it finish reading. Configuration will close InputStream, just as the javadoc annotation. Thank you again for your review. > Add HDFSSchedulerConfigurationStore for RM HA > --------------------------------------------- > > Key: YARN-7497 > URL: https://issues.apache.org/jira/browse/YARN-7497 > Project: Hadoop YARN > Issue Type: Sub-task > Components: yarn > Reporter: Jiandan Yang > Assignee: Jiandan Yang > Priority: Major > Attachments: YARN-7497.001.patch, YARN-7497.002.patch, > YARN-7497.003.patch, YARN-7497.004.patch, YARN-7497.005.patch, > YARN-7497.006.patch, YARN-7497.007.patch, YARN-7497.008.patch, > YARN-7497.009.patch > > > YARN-5947 add LeveldbConfigurationStore using Leveldb as backing store, but > it does not support Yarn RM HA. > YARN-6840 supports RM HA, but too many scheduler configurations may exceed > znode limit, for example 10 thousand queues. > HDFSSchedulerConfigurationStore store conf file in HDFS, when RM failover, > new active RM can load scheduler configuration from HDFS. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org