> On March 15, 2014, 11:39 p.m., Jihoon Son wrote:
> > Hi, Dae Myung. Many thanks for your report and patch.
> >
> > I have a question about your report.
> > TajoMaster and CatalogServer extend CompositeService and AbstractService,
> > respectively.
> > When super.init() is called in TajoMaster.init(), CatalogServer.init() is
> > called via CompositeService.serviceInit().
> > As you can see, the configuration looks to be assigned in
> > CatalogServer.init().
> > So, I think that the configuration is assigned successfully.
> > (When I check the log of TajoMaster, I found the following line.
> > 2014-03-10 09:09:35,297 INFO catalog.CatalogServer
> > (CatalogServer.java:init(111)) - Catalog Store Class:
> > org.apache.tajo.catalog.store.DerbyStore)
> > If I missed any other problems during the initialization of CatalogServer,
> > please correct me.
> >
> > Thanks,
> > Jihoon
>
> Dae Myung Kang wrote:
> @Jihoon Yes. you're almost right. I just found some irregular case. and
> It teased me.
>
> finally. CatalogServer.init() is called. and that time catalogServer can
> get tajoConf.
>
> but we should see that parts. it is "public void init(Configuration
> _conf)" in TajoMaster.java
>
> ```java
> catalogServer = new CatalogServer(initBuiltinFunctions(),
> systemConf);
> addIfService(catalogServer);
> catalog = new LocalCatalogWrapper(catalogServer);
> ```
>
> before calling catalogServer.init() function(in CompositeService),
> LocalCatalogWrapper already uses tajoConf like below in its Constructor.
>
> ```java
> public LocalCatalogWrapper(final CatalogServer server) {
> super(server.getConf(), null);
> this.catalog = server;
> this.stub = server.getHandler();
> }
>
> ```
>
> so it can cause null pointer exception if someone call tajoconf from
> LocalCatalogWrapper.
>
> Thank you.
>
Thanks for the further explanation. I missed that part.
Your patch looks to be enough to solve the aforementioned problem.
But, TajoConf may be assigned repeatedly (in the constructor and the init()
function), and it will be definitely an unnecessary operation.
Would you improve the patch to handle this repeated assignment?
- Jihoon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19221/#review37317
-----------------------------------------------------------
On March 14, 2014, 10:27 p.m., Dae Myung Kang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19221/
> -----------------------------------------------------------
>
> (Updated March 14, 2014, 10:27 p.m.)
>
>
> Review request for Tajo.
>
>
> Bugs: https://issues.apache.org/jira/browse/TAJO-687
>
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/TAJO-687
>
>
> Repository: tajo
>
>
> Description
> -------
>
>
> TajoMaster currently doesn't pass tajoConf to create catalogServer. so it
> create RpcConnectionPool with null tajoConf.
>
> so it can cause NullPointerException if you use tajo conf in NettyClientBase
>
>
> Diffs
> -----
>
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
> 621b475
>
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java
> c42b0f3
>
> Diff: https://reviews.apache.org/r/19221/diff/
>
>
> Testing
> -------
>
> pass "mvn clean install"
>
>
> Thanks,
>
> Dae Myung Kang
>
>
