[CARBONDATA-2648] Fixed NPE issue with legacy store when CACHE_LEVEL is Blocklet
Things done as part of this PR: Fixed Null pointer exception when store is of <= 1.1 version and DataMap is of type BlockletDataMap. Added clearing of SegmentProperties cache holder from executor Problem 1: Null pointer exception thrown when store is of <= 1.1 version and DataMap is of type BlockletDataMap. Analysis: In BlcokletDataMap schema is created to consider blockletInfo while adding to unsafe but in case of legacy store we dont weite the blocklet Info. This lead to Null pointer exception while calculating the size of row during adding to unsafe. Solution For legacy store always call super class methods which is BlockDataMap. This closes #2499 Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/bd02656a Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/bd02656a Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/bd02656a Branch: refs/heads/carbonstore Commit: bd02656abc559040111c636c909541c8441b8132 Parents: 1fd3703 Author: manishgupta88 <tomanishgupt...@gmail.com> Authored: Thu Jul 12 21:35:45 2018 +0530 Committer: ravipesala <ravi.pes...@gmail.com> Committed: Fri Jul 13 13:58:46 2018 +0530 ---------------------------------------------------------------------- .../core/datamap/DistributableDataMapFormat.java | 4 ++++ .../indexstore/BlockletDataMapIndexStore.java | 9 +++++---- .../TableBlockIndexUniqueIdentifierWrapper.java | 18 ++++++++++++++++++ .../indexstore/blockletindex/BlockletDataMap.java | 14 +++++++++++++- .../core/indexstore/row/DataMapRow.java | 5 +++++ 5 files changed, 45 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/carbondata/blob/bd02656a/core/src/main/java/org/apache/carbondata/core/datamap/DistributableDataMapFormat.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/core/datamap/DistributableDataMapFormat.java b/core/src/main/java/org/apache/carbondata/core/datamap/DistributableDataMapFormat.java index 762d89c..007541d 100644 --- a/core/src/main/java/org/apache/carbondata/core/datamap/DistributableDataMapFormat.java +++ b/core/src/main/java/org/apache/carbondata/core/datamap/DistributableDataMapFormat.java @@ -26,6 +26,7 @@ import java.util.List; import org.apache.carbondata.core.datamap.dev.DataMap; import org.apache.carbondata.core.datamap.dev.expr.DataMapDistributableWrapper; import org.apache.carbondata.core.datamap.dev.expr.DataMapExprWrapper; +import org.apache.carbondata.core.datastore.block.SegmentPropertiesAndSchemaHolder; import org.apache.carbondata.core.indexstore.ExtendedBlocklet; import org.apache.carbondata.core.indexstore.PartitionSpec; import org.apache.carbondata.core.metadata.schema.table.CarbonTable; @@ -97,6 +98,9 @@ public class DistributableDataMapFormat extends FileInputFormat<Void, ExtendedBl // if job is to clear datamaps just clear datamaps from cache and return DataMapStoreManager.getInstance() .clearDataMaps(table.getCarbonTableIdentifier().getTableUniqueName()); + // clear the segment properties cache from executor + SegmentPropertiesAndSchemaHolder.getInstance() + .invalidate(table.getAbsoluteTableIdentifier()); blockletIterator = Collections.emptyIterator(); return; } http://git-wip-us.apache.org/repos/asf/carbondata/blob/bd02656a/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java b/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java index 33e624d..d84f977 100644 --- a/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java +++ b/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java @@ -93,7 +93,7 @@ public class BlockletDataMapIndexStore carbonDataFileBlockMetaInfoMapping); BlockDataMap blockletDataMap = loadAndGetDataMap(identifier, indexFileStore, blockMetaInfoMap, - identifierWrapper.getCarbonTable()); + identifierWrapper.getCarbonTable(), identifierWrapper.isAddTableBlockToUnsafe()); dataMaps.add(blockletDataMap); blockletDataMapIndexWrapper = new BlockletDataMapIndexWrapper(dataMaps); } else { @@ -108,7 +108,8 @@ public class BlockletDataMapIndexStore carbonDataFileBlockMetaInfoMapping); BlockDataMap blockletDataMap = loadAndGetDataMap(blockIndexUniqueIdentifier, indexFileStore, blockMetaInfoMap, - identifierWrapper.getCarbonTable()); + identifierWrapper.getCarbonTable(), + identifierWrapper.isAddTableBlockToUnsafe()); dataMaps.add(blockletDataMap); } blockletDataMapIndexWrapper = new BlockletDataMapIndexWrapper(dataMaps); @@ -251,7 +252,7 @@ public class BlockletDataMapIndexStore */ private BlockDataMap loadAndGetDataMap(TableBlockIndexUniqueIdentifier identifier, SegmentIndexFileStore indexFileStore, Map<String, BlockMetaInfo> blockMetaInfoMap, - CarbonTable carbonTable) + CarbonTable carbonTable, boolean addTableBlockToUnsafe) throws IOException, MemoryException { String uniqueTableSegmentIdentifier = identifier.getUniqueTableSegmentIdentifier(); @@ -265,7 +266,7 @@ public class BlockletDataMapIndexStore dataMap.init(new BlockletDataMapModel(carbonTable, identifier.getIndexFilePath() + CarbonCommonConstants.FILE_SEPARATOR + identifier .getIndexFileName(), indexFileStore.getFileData(identifier.getIndexFileName()), - blockMetaInfoMap, identifier.getSegmentId())); + blockMetaInfoMap, identifier.getSegmentId(), addTableBlockToUnsafe)); } return dataMap; } http://git-wip-us.apache.org/repos/asf/carbondata/blob/bd02656a/core/src/main/java/org/apache/carbondata/core/indexstore/TableBlockIndexUniqueIdentifierWrapper.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/core/indexstore/TableBlockIndexUniqueIdentifierWrapper.java b/core/src/main/java/org/apache/carbondata/core/indexstore/TableBlockIndexUniqueIdentifierWrapper.java index 3411397..0924f1f 100644 --- a/core/src/main/java/org/apache/carbondata/core/indexstore/TableBlockIndexUniqueIdentifierWrapper.java +++ b/core/src/main/java/org/apache/carbondata/core/indexstore/TableBlockIndexUniqueIdentifierWrapper.java @@ -35,6 +35,10 @@ public class TableBlockIndexUniqueIdentifierWrapper implements Serializable { // holds the reference to CarbonTable private CarbonTable carbonTable; + /** + * flag to specify whether to load table block metadata in unsafe or safe. Default value is true + */ + private boolean addTableBlockToUnsafe = true; public TableBlockIndexUniqueIdentifierWrapper( TableBlockIndexUniqueIdentifier tableBlockIndexUniqueIdentifier, CarbonTable carbonTable) { @@ -42,6 +46,16 @@ public class TableBlockIndexUniqueIdentifierWrapper implements Serializable { this.carbonTable = carbonTable; } + // Note: The constructor is getting used in extensions with other functionalities. + // Kindly do not remove + public TableBlockIndexUniqueIdentifierWrapper( + TableBlockIndexUniqueIdentifier tableBlockIndexUniqueIdentifier, CarbonTable carbonTable, + boolean addTableBlockToUnsafe) { + this(tableBlockIndexUniqueIdentifier, carbonTable); + this.addTableBlockToUnsafe = addTableBlockToUnsafe; + } + + public TableBlockIndexUniqueIdentifier getTableBlockIndexUniqueIdentifier() { return tableBlockIndexUniqueIdentifier; } @@ -49,4 +63,8 @@ public class TableBlockIndexUniqueIdentifierWrapper implements Serializable { public CarbonTable getCarbonTable() { return carbonTable; } + + public boolean isAddTableBlockToUnsafe() { + return addTableBlockToUnsafe; + } } http://git-wip-us.apache.org/repos/asf/carbondata/blob/bd02656a/core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java b/core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java index d53e936..bbe37c0 100644 --- a/core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java +++ b/core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMap.java @@ -73,6 +73,9 @@ public class BlockletDataMap extends BlockDataMap implements Serializable { } protected CarbonRowSchema[] getTaskSummarySchema() { + if (isLegacyStore) { + return super.getTaskSummarySchema(); + } SegmentPropertiesAndSchemaHolder.SegmentPropertiesWrapper segmentPropertiesWrapper = SegmentPropertiesAndSchemaHolder.getInstance() .getSegmentPropertiesWrapper(segmentPropertiesIndex); @@ -84,6 +87,9 @@ public class BlockletDataMap extends BlockDataMap implements Serializable { } protected CarbonRowSchema[] getFileFooterEntrySchema() { + if (isLegacyStore) { + return super.getFileFooterEntrySchema(); + } return SegmentPropertiesAndSchemaHolder.getInstance() .getSegmentPropertiesWrapper(segmentPropertiesIndex).getBlockletFileFooterEntrySchema(); } @@ -199,7 +205,7 @@ public class BlockletDataMap extends BlockDataMap implements Serializable { public ExtendedBlocklet getDetailedBlocklet(String blockletId) { if (isLegacyStore) { - super.getDetailedBlocklet(blockletId); + return super.getDetailedBlocklet(blockletId); } int absoluteBlockletId = Integer.parseInt(blockletId); DataMapRow safeRow = memoryDMStore.getDataMapRow(getFileFooterEntrySchema(), absoluteBlockletId) @@ -210,10 +216,16 @@ public class BlockletDataMap extends BlockDataMap implements Serializable { } protected short getBlockletId(DataMapRow dataMapRow) { + if (isLegacyStore) { + return super.getBlockletId(dataMapRow); + } return dataMapRow.getShort(BLOCKLET_ID_INDEX); } protected ExtendedBlocklet createBlocklet(DataMapRow row, String fileName, short blockletId) { + if (isLegacyStore) { + return super.createBlocklet(row, fileName, blockletId); + } ExtendedBlocklet blocklet = new ExtendedBlocklet(fileName, blockletId + ""); BlockletDetailInfo detailInfo = getBlockletDetailInfo(row, blockletId, blocklet); detailInfo.setColumnSchemas(getColumnSchema()); http://git-wip-us.apache.org/repos/asf/carbondata/blob/bd02656a/core/src/main/java/org/apache/carbondata/core/indexstore/row/DataMapRow.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/core/indexstore/row/DataMapRow.java b/core/src/main/java/org/apache/carbondata/core/indexstore/row/DataMapRow.java index e9ce170..d9467c6 100644 --- a/core/src/main/java/org/apache/carbondata/core/indexstore/row/DataMapRow.java +++ b/core/src/main/java/org/apache/carbondata/core/indexstore/row/DataMapRow.java @@ -86,6 +86,11 @@ public abstract class DataMapRow implements Serializable { case VARIABLE_INT: return getLengthInBytes(ordinal) + 4; case STRUCT: + DataMapRow row = getRow(ordinal); + CarbonRowSchema[] childSchemas = + ((CarbonRowSchema.StructCarbonRowSchema) schemas[ordinal]).getChildSchemas(); + // set the child schema. Because schema is transient it can be null + row.setSchemas(childSchemas); return getRow(ordinal).getTotalSizeInBytes(); default: throw new UnsupportedOperationException("wrong type");
