Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/214#issuecomment-60572743
> Hello @hyunsik ,
I will fix some missing points for your comments, and will rebase it.
On my implementation, I have overrided some functions of Configuration
class to raise exceptions when invalid values are inserted. However, with your
explanation, I feel that TajoMaster will throw exceptions, not TajoConf. I just
wonder if I correctly understand your strategy.
Hi @ykrips,
Thank you for your work again! I leave comments here for your question in
#206.
The main reason why TajoMaster needs to check all configs is that we need
to limit the scope of runtime exception.
The background is as follows:
* TajoConf is a subclass of Hadoop Configuration.
* Configuration loads ```*-site.xml``` and ```*-default.xml``` in Java
classpaths when it is initialized.
* Coniguration internally will use set(String, String) when it
initialized.
* According to our policy of TajoConf usage, we **do not modify any config
in runtime** except for unit tests.
* In other words, they are constant configs loaded when Tajo cluster
starts up.
* As a result, validateProperty() will be mostly used when TajoConf is
initialized.
Consequently, we need to handle exception thrown by the constructor of
TajoConf. TajoConf is initialized in lots of parts, and exception handling
against constructor may be not good in my opinion.
In my view. it would be enough to validate configs in three entry points of
TajoMaster, TajoWorker and TajoClient. They are main components in a Tajo
cluster. (In previous, I mentioned only TajoMaster. We actually need config
validation in two more components.)
Later, we can improve three components (TajoMaster, TajoWorker, and
TajoClient) to have some diagnose phase to check all configs, connectivities
among Tajo components and states of components. Your proposed patch would be a
part of the diagnose phase.
Thanks,
Hyunsik
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
