[GitHub] carbondata pull request #1306: [CARBONDATA-1436] optimize concurrent control...

[qiuchenjian-2]

GitHub user xuchuanyin opened a pull request:

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

    [CARBONDATA-1436] optimize concurrent control for datamap

    # Scenario
   
    `DataMapStoreManager` provides a synchronized interface `getDataMap` to retrieve a table's `TableDataMap`. It will cause performance problems in current query scenario when all the queries have to wait the former query finished.
   
    # Analyze
   
    We can make the concurrent control in table scope instead of global scope.
   
    # Modification
   
    + Synchronized by table.
   
    + Use double checked locking to reduce lock overhead

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

    $ git pull https://github.com/xuchuanyin/carbondata opt_datamap_concurrency

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

    https://github.com/apache/carbondata/pull/1306.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 #1306
   
----
commit 36e48808132f3d566aff102d82524767f509e12d
Author: xuchuanyin <[hidden email]>
Date:   2017-08-31T06:08:45Z

    optimize concurrency control for datamap

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1306#discussion_r136254674
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java ---
    @@ -82,7 +90,6 @@ public TableDataMap createAndRegisterDataMap(AbsoluteTableIdentifier identifier,
         List<TableDataMap> tableDataMaps = allDataMaps.get(table);
         if (tableDataMaps == null) {
           tableDataMaps = new ArrayList<>();
    -      allDataMaps.put(table, tableDataMaps);
    --- End diff --
   
    FYI:
   
    should move this line to the end, otherwise in concurrent scenario, the other thread will get empty (not null) `tableMaps` in `getDataMap`, which will result in `Datamap does not exist` exception.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1306
 
    SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/351/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1306
 
    SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/355/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/1306
 
    LGTM


---