[GitHub] carbondata pull request #2178: show table in presto doesn't initialized load...

[qiuchenjian-2]

GitHub user photogamerun opened a pull request:

    https://github.com/apache/carbondata/pull/2178

    show table in presto doesn't initialized load carbon metadata

    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/photogamerun/carbondata-1 master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/2178.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2178
   
----
commit 71fe3a32ea010e28a280672172535fcf458ee6f9
Author: photogamerun <27547073@...>
Date:   2018-04-17T09:18:00Z

    fix the firstly show table without load hdfs metdata file

commit 717ba0e23779c069fbf9a67c6224a07be2503b3a
Author: photogamerun <27547073@...>
Date:   2018-04-17T09:21:05Z

    fix the firstly show table without load hdfs metdata file

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Can one of the admins verify this patch?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Can one of the admins verify this patch?


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Please add jira number in title.
    Can you provide more infomation?


---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Please add the steps to reproduce in PR description @photogamerun


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Can one of the admins verify this patch?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Can one of the admins verify this patch?


---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    @photogamerun  please create a jira ticket for this pr and please provide more detail about this pr.


---

[qiuchenjian-2]

Github user photogamerun commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    firstly  let me explain this change
   
     private Set<String> updateTableList(String schemaName, String user) {
    updateCarbonFile(user);
   
    private CarbonFile carbonFileList; is initially null
    only this method updateCarbonFile give the value to carbonFileList
   
    so once user call updateTableList(user) without initilize carbonFileList it will throw nullpointexception in updateTableList method
     
    List<CarbonFile> schema =
                Stream.of(carbonFileList.listFiles()).filter(a -> schemaName.equals(a.getName()))
                        .collect(Collectors.toList());
   
   



---

[qiuchenjian-2]

Github user photogamerun commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    请看下 Files changed 中的文件的改变(please check the diff in Files changed)
   
    前面的回复是对于第一个方法updateTableList 需要在首先调用updateCarbonFile  æ–¹æ³•åŽ»åˆå§‹åŒ–carbonFileList这个属性 否则，会出空指针(the above reply is for the first change we need to call updateCarbonFile   before the logic is executed in updateTableList   method )
   



---

[qiuchenjian-2]

Github user photogamerun commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    第二个问题 change 是这样的
    parseCarbonMetadata 这个方法我们发现存在多线程去调用
   
    当时抛了一个这样的异常信息
    tableCacheModel.carbonTable should not be null
    后来通过分析代码发现并发模式下，tableCacheModel.carbonTable 中的carbonTable 对象在没有获取前 就已经被另外的线程获取并且读取了。 所以存在读不到表信息的bug
   
    所以在之前实现中有这样一个问题parseCarbonMetadata 中
   
    carbonCache.get().put(table, cache);先调用，
   
    然后走了一个堆逻辑再对cache中的属性做赋值
   
    cache.tableInfo = wrapperTableInfo;
    cache.carbonTable = CarbonMetadata.getInstance()
                  .getCarbonTable(cache.carbonTableIdentifier.getTableUniqueName());
   
    这样存在一个严重问题，在放入cache 和属性赋值的这个时间段内。
    一旦有方法调用carbonCache.get().get(table);
    比如这个方法
   
     public List<CarbonLocalInputSplit> getInputSplits2(CarbonTableCacheModel tableCacheModel,
                                                         Expression filters, Expression partitionFilters,
                                                         ConnectorSession session)  {
    private void removeTableFromCache(SchemaTableName table) {
   
    他们在同时时间都会调用 carbonCache.get().get(table);继而拿取内部属性carbonTable 并调用其api 这个时间其实carbonTable 这个属性是null 调用其api一定会出问题
   
   
   
   
   
   



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    add to whitelist



---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2178#discussion_r200978774
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/impl/CarbonTableReader.java ---
    @@ -314,7 +315,7 @@ private CarbonTable loadTableMetadata(SchemaTableName schemaTableName) {
         for (SchemaTableName table : tableList) {
           if (!table.equals(schemaTableName)) continue;
     
    -      return parseCarbonMetadata(table);
    +      return parseCarbonMetadata(table).carbonTable;
    --- End diff --
   
    why here need to add ".carbonTable"?
   
    Inside parseCarbonMetadata(table), already have the below code 👍
    result = cache.carbonTable;


---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2178#discussion_r200981549
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/impl/CarbonTableReader.java ---
    @@ -325,14 +326,23 @@ private CarbonTable loadTableMetadata(SchemaTableName schemaTableName) {
        * @param table name of the given table.
        * @return the CarbonTable instance which contains all the needed metadata for a table.
    --- End diff --
   
    please update the comment info also. now is not returning CarbonTable instance.


---

[qiuchenjian-2]

Github user photogamerun commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    please refer to https://github.com/apache/carbondata/pull/2468


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6957/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6961/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5741/



---

[qiuchenjian-2]

Github user photogamerun closed the pull request at:

    https://github.com/apache/carbondata/pull/2178


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2178
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5750/



---