GitHub user jackylk opened a pull request:
https://github.com/apache/incubator-carbondata/pull/472 [CARBONDATA] clean up code for carbon-core module clean up: 1. remove unused declaration 2.remove unnecessary exception You can merge this pull request into a Git repository by running: $ git pull https://github.com/jackylk/incubator-carbondata cleancore Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-carbondata/pull/472.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 #472 ---- commit f093d70e5d9f97b0f4e8b8f93ac8c0f9e62caa64 Author: jackylk <[hidden email]> Date: 2016-12-27T12:42:07Z remove unused declare commit 9e20b09a68a6e86fa072a0a58519e6167191cc33 Author: jackylk <[hidden email]> Date: 2016-12-27T16:11:35Z fix style commit 0419cf5988f28f2aa9737455ad255ac87841775f Author: jackylk <[hidden email]> Date: 2016-12-27T16:53:04Z clean up commit 391e1efd340b4ebdebfad94bca82ea33be4a4954 Author: jackylk <[hidden email]> Date: 2016-12-27T17:11:40Z fix style ---- --- 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. --- |
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/343/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/344/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/345/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/346/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/378/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/380/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/381/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/382/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/383/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/384/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/385/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/386/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user QiangCai commented on a diff in the pull request:
https://github.com/apache/incubator-carbondata/pull/472#discussion_r94367280 --- Diff: core/src/main/java/org/apache/carbondata/core/carbon/datastore/chunk/reader/dimension/v2/CompressedDimensionChunkFileBasedReaderV2.java --- @@ -135,6 +135,7 @@ public CompressedDimensionChunkFileBasedReaderV2(final BlockletInfo blockletInfo dimensionChunk = fileReader.readByteArray(filePath, dimensionChunksOffset.get(blockIndex), dimensionChunksLength.get(blockIndex)); dimensionColumnChunk = CarbonUtil.readDataChunk(dimensionChunk); + assert dimensionColumnChunk != null; --- End diff -- I prefer to throw an 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. --- |
In reply to this post by qiuchenjian-2
Github user QiangCai commented on a diff in the pull request:
https://github.com/apache/incubator-carbondata/pull/472#discussion_r94366862 --- Diff: core/src/main/java/org/apache/carbondata/core/cache/dictionary/AbstractDictionaryCache.java --- @@ -149,65 +147,61 @@ private CarbonFile getDictionaryMetaCarbonFile( * tableName and columnIdentifier * @param dictionaryInfo * @param lruCacheKey - * @param loadSortIndex read and load sort index file in memory - * @throws CarbonUtilException in case memory is not sufficient to load dictionary into memory + * @param IOException in case memory is not sufficient to load dictionary + * into memory */ protected void checkAndLoadDictionaryData( DictionaryColumnUniqueIdentifier dictionaryColumnUniqueIdentifier, DictionaryInfo dictionaryInfo, String lruCacheKey, boolean loadSortIndex) - throws CarbonUtilException { - try { - // read last segment dictionary meta chunk entry to get the end offset of file - CarbonFile carbonFile = getDictionaryMetaCarbonFile(dictionaryColumnUniqueIdentifier); - boolean dictionaryMetaFileModified = - isDictionaryMetaFileModified(carbonFile, dictionaryInfo.getFileTimeStamp(), - dictionaryInfo.getDictionaryMetaFileLength()); - // if dictionary metadata file is modified then only read the last entry from dictionary - // meta file - if (dictionaryMetaFileModified) { - synchronized (dictionaryInfo) { - carbonFile = getDictionaryMetaCarbonFile(dictionaryColumnUniqueIdentifier); - dictionaryMetaFileModified = - isDictionaryMetaFileModified(carbonFile, dictionaryInfo.getFileTimeStamp(), - dictionaryInfo.getDictionaryMetaFileLength()); - // Double Check : - // if dictionary metadata file is modified then only read the last entry from dictionary - // meta file - if (dictionaryMetaFileModified) { - CarbonDictionaryColumnMetaChunk carbonDictionaryColumnMetaChunk = - readLastChunkFromDictionaryMetadataFile(dictionaryColumnUniqueIdentifier); - // required size will be size total size of file - offset till file is - // already read - long requiredSize = - carbonDictionaryColumnMetaChunk.getEnd_offset() - dictionaryInfo.getMemorySize(); - if (requiredSize > 0) { - boolean columnAddedToLRUCache = - carbonLRUCache.put(lruCacheKey, dictionaryInfo, requiredSize); - // if column is successfully added to lru cache then only load the - // dictionary data - if (columnAddedToLRUCache) { - // load dictionary data - loadDictionaryData(dictionaryInfo, dictionaryColumnUniqueIdentifier, - dictionaryInfo.getMemorySize(), carbonDictionaryColumnMetaChunk.getEnd_offset(), - loadSortIndex); - // set the end offset till where file is read - dictionaryInfo - .setOffsetTillFileIsRead(carbonDictionaryColumnMetaChunk.getEnd_offset()); - dictionaryInfo.setFileTimeStamp(carbonFile.getLastModifiedTime()); - dictionaryInfo.setDictionaryMetaFileLength(carbonFile.getSize()); - } else { - throw new CarbonUtilException( - "Cannot load dictionary into memory. Not enough memory available"); - } + throws IOException { + // read last segment dictionary meta chunk entry to get the end offset of file + CarbonFile carbonFile = getDictionaryMetaCarbonFile(dictionaryColumnUniqueIdentifier); + boolean dictionaryMetaFileModified = + isDictionaryMetaFileModified(carbonFile, dictionaryInfo.getFileTimeStamp(), + dictionaryInfo.getDictionaryMetaFileLength()); + // if dictionary metadata file is modified then only read the last entry from dictionary + // meta file + if (dictionaryMetaFileModified) { + synchronized (dictionaryInfo) { + carbonFile = getDictionaryMetaCarbonFile(dictionaryColumnUniqueIdentifier); + dictionaryMetaFileModified = + isDictionaryMetaFileModified(carbonFile, dictionaryInfo.getFileTimeStamp(), + dictionaryInfo.getDictionaryMetaFileLength()); + // Double Check : + // if dictionary metadata file is modified then only read the last entry from dictionary + // meta file + if (dictionaryMetaFileModified) { + CarbonDictionaryColumnMetaChunk carbonDictionaryColumnMetaChunk = + readLastChunkFromDictionaryMetadataFile(dictionaryColumnUniqueIdentifier); + // required size will be size total size of file - offset till file is + // already read + long requiredSize = + carbonDictionaryColumnMetaChunk.getEnd_offset() - dictionaryInfo.getMemorySize(); + if (requiredSize > 0) { + boolean columnAddedToLRUCache = + carbonLRUCache.put(lruCacheKey, dictionaryInfo, requiredSize); + // if column is successfully added to lru cache then only load the + // dictionary data + if (columnAddedToLRUCache) { + // load dictionary data + loadDictionaryData(dictionaryInfo, dictionaryColumnUniqueIdentifier, + dictionaryInfo.getMemorySize(), carbonDictionaryColumnMetaChunk.getEnd_offset(), + loadSortIndex); + // set the end offset till where file is read + dictionaryInfo + .setOffsetTillFileIsRead(carbonDictionaryColumnMetaChunk.getEnd_offset()); + dictionaryInfo.setFileTimeStamp(carbonFile.getLastModifiedTime()); + dictionaryInfo.setDictionaryMetaFileLength(carbonFile.getSize()); + } else { + throw new IOException( --- End diff -- Why not use custom exception class? --- 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. --- |
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/incubator-carbondata/pull/472#discussion_r94426918 --- Diff: core/src/main/java/org/apache/carbondata/core/carbon/datastore/chunk/reader/dimension/v2/CompressedDimensionChunkFileBasedReaderV2.java --- @@ -135,6 +135,7 @@ public CompressedDimensionChunkFileBasedReaderV2(final BlockletInfo blockletInfo dimensionChunk = fileReader.readByteArray(filePath, dimensionChunksOffset.get(blockIndex), dimensionChunksLength.get(blockIndex)); dimensionColumnChunk = CarbonUtil.readDataChunk(dimensionChunk); + assert dimensionColumnChunk != null; --- End diff -- Even I feel throwing custom exception is better. --- 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. --- |
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/incubator-carbondata/pull/472#discussion_r94521856 --- Diff: core/src/main/java/org/apache/carbondata/core/carbon/datastore/chunk/reader/dimension/v2/CompressedDimensionChunkFileBasedReaderV2.java --- @@ -135,6 +135,7 @@ public CompressedDimensionChunkFileBasedReaderV2(final BlockletInfo blockletInfo dimensionChunk = fileReader.readByteArray(filePath, dimensionChunksOffset.get(blockIndex), dimensionChunksLength.get(blockIndex)); dimensionColumnChunk = CarbonUtil.readDataChunk(dimensionChunk); + assert dimensionColumnChunk != null; --- End diff -- ok. And I found that FileHolder interface is ignoring IOException (catched but just log, not throwing it). So now I changed FileHolder and the caller to throw IOException --- 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. --- |
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/incubator-carbondata/pull/472#discussion_r94533501 --- Diff: core/src/main/java/org/apache/carbondata/core/carbon/datastore/chunk/reader/dimension/v2/CompressedDimensionChunkFileBasedReaderV2.java --- @@ -135,6 +135,7 @@ public CompressedDimensionChunkFileBasedReaderV2(final BlockletInfo blockletInfo dimensionChunk = fileReader.readByteArray(filePath, dimensionChunksOffset.get(blockIndex), dimensionChunksLength.get(blockIndex)); dimensionColumnChunk = CarbonUtil.readDataChunk(dimensionChunk); + assert dimensionColumnChunk != null; --- End diff -- ok --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Failed with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/430/ --- 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. --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/incubator-carbondata/pull/472 Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/432/ --- 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. --- |
Free forum by Nabble | Edit this page |