You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by dhatchayani <gi...@git.apache.org> on 2018/07/20 14:37:51 UTC

[GitHub] carbondata pull request #2534: [CARBONDATA-2753] Fix Compatibility issues on...

GitHub user dhatchayani opened a pull request:

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

    [CARBONDATA-2753] Fix Compatibility issues on index Files

    **Problem:**
    Currently,in the segmentFile we are writing the index files list in files field, only if it exists, otherwise it will be empty(in case if it is merged to merge index file). But in the old store, we were writing both the files and mergeFileName fields even if the index files are merged.
    
    **Solution:**
    While querying we have to check the physical existence of the index files listed in the files field. If it physically exists, then we have to consider that.
    
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
    
     - [x] Testing done
            Manual Testing with Old Store
           
     - [ ] 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/dhatchayani/carbondata CARBONDATA-2753_indexFiles

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

    https://github.com/apache/carbondata/pull/2534.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 #2534
    
----
commit e257177c182783fb6c78507d4d8ad9823022acb6
Author: dhatchayani <dh...@...>
Date:   2018-07-20T14:36:00Z

    [CARBONDATA-2753] Fix Compatibility issues on index Files

----


---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by dhatchayani <gi...@git.apache.org>.
Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    Retest this please


---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by dhatchayani <gi...@git.apache.org>.
Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    retest this please


---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues while que...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues while que...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5945/



---

[GitHub] carbondata pull request #2534: [CARBONDATA-2753] Fix Compatibility issues wh...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5941/



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by brijoobopanna <gi...@git.apache.org>.
Github user brijoobopanna commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    retest sdv please



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues while que...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5960/



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6128/



---

[GitHub] carbondata pull request #2534: [CARBONDATA-2753] Fix Compatibility issues on...

Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2534#discussion_r204287054
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java ---
    @@ -597,7 +597,14 @@ public SegmentFile getSegmentFile() {
               Set<String> files = entry.getValue().getFiles();
               if (null != files && !files.isEmpty()) {
                 for (String indexFile : files) {
    -              indexFiles.put(location + CarbonCommonConstants.FILE_SEPARATOR + indexFile, null);
    +              String indexFilePath = location + CarbonCommonConstants.FILE_SEPARATOR + indexFile;
    +              try {
    +                if (FileFactory.isFileExist(indexFilePath)) {
    +                  indexFiles.put(indexFilePath, null);
    +                }
    +              } catch (IOException e) {
    +                LOGGER.error(e.getMessage());
    --- End diff --
    
    1. Don't LOG the error here, throw IO exception back to caller
    2. Mention the compatibility version number and provide a detailed comment in this method for file Existence check including the version numbers


---

[GitHub] carbondata pull request #2534: [CARBONDATA-2753] Fix Compatibility issues on...

Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2534#discussion_r204287026
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java ---
    @@ -597,7 +597,14 @@ public SegmentFile getSegmentFile() {
               Set<String> files = entry.getValue().getFiles();
               if (null != files && !files.isEmpty()) {
                 for (String indexFile : files) {
    -              indexFiles.put(location + CarbonCommonConstants.FILE_SEPARATOR + indexFile, null);
    +              String indexFilePath = location + CarbonCommonConstants.FILE_SEPARATOR + indexFile;
    +              try {
    +                if (FileFactory.isFileExist(indexFilePath)) {
    +                  indexFiles.put(indexFilePath, null);
    +                }
    +              } catch (IOException e) {
    +                LOGGER.error(e.getMessage());
    --- End diff --
    
    1. Don't LOG the error here, throw IO exception back to caller
    2. Mention the compatibility version number and provide a detailed comment in this method for file Existence check including the version numbers


---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2534
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6131/



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues on index ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2534: [CARBONDATA-2753] Fix Compatibility issues while que...

Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:

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


---