kunal642 commented on a change in pull request #3988:
URL: https://github.com/apache/carbondata/pull/3988#discussion_r516152662
##########
File path:
core/src/main/java/org/apache/carbondata/core/readcommitter/TableStatusReadCommittedScope.java
##########
@@ -79,13 +83,24 @@ public
TableStatusReadCommittedScope(AbsoluteTableIdentifier identifier,
@Override
public Map<String, String> getCommittedIndexFile(Segment segment) throws
IOException {
Map<String, String> indexFiles;
- if (segment.getSegmentFileName() == null) {
+ SegmentFileStore fileStore =
Review comment:
based on the old check if segmentFileName can be null then this code can
fail..Please check this
##########
File path:
core/src/main/java/org/apache/carbondata/core/readcommitter/TableStatusReadCommittedScope.java
##########
@@ -79,13 +83,24 @@ public
TableStatusReadCommittedScope(AbsoluteTableIdentifier identifier,
@Override
public Map<String, String> getCommittedIndexFile(Segment segment) throws
IOException {
Map<String, String> indexFiles;
- if (segment.getSegmentFileName() == null) {
+ SegmentFileStore fileStore =
+ new SegmentFileStore(identifier.getTablePath(),
segment.getSegmentFileName());
+ if (fileStore.getSegmentFile() == null) {
String path =
CarbonTablePath.getSegmentPath(identifier.getTablePath(),
segment.getSegmentNo());
indexFiles = new
SegmentIndexFileStore().getMergeOrIndexFilesFromSegment(path);
+ List<String> indexFileList = new ArrayList<>(indexFiles.keySet());
+ Set<String> mergedIndexFiles =
+ SegmentFileStore.getInvalidAndMergedIndexFiles(indexFileList);
+ for (String indexFile : indexFileList) {
+ if (mergedIndexFiles
+
.contains(indexFile.substring(indexFile.lastIndexOf(File.separator) + 1))) {
+ // do not include already merged index files details.
+ indexFiles.remove(indexFile);
+ }
+ }
+ indexFileList = null;
Review comment:
this is not needed
##########
File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java
##########
@@ -2558,8 +2558,8 @@ public static long getCarbonIndexSize(SegmentFileStore
fileStore,
// Get the total size of carbon data and the total size of carbon index
public static HashMap<String, Long> getDataSizeAndIndexSize(String tablePath,
Segment segment) throws IOException {
- if (segment.getSegmentFileName() != null) {
- SegmentFileStore fileStore = new SegmentFileStore(tablePath,
segment.getSegmentFileName());
+ SegmentFileStore fileStore = new SegmentFileStore(tablePath,
segment.getSegmentFileName());
+ if (fileStore.getSegmentFile() != null) {
Review comment:
same as previous check..Please check if this will fail when
segmmentFileName is null
##########
File path:
core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/SegmentIndexFileStore.java
##########
@@ -79,6 +79,11 @@
*/
private Map<String, List<String>> carbonMergeFileToIndexFilesMap;
+ /**
+ * Stores the list of invalid index files of the SI segments in case of
small files.
+ */
+ private static Set<String> oldSIIndexAndMergeIndexFiles = new HashSet<>();
Review comment:
This would fail in case of concurrent queries as 1 operation may
override for another
##########
File path:
core/src/main/java/org/apache/carbondata/core/mutate/CarbonUpdateUtil.java
##########
@@ -274,20 +274,22 @@ public static boolean
updateTableMetadataStatus(Set<Segment> updatedSegmentsList
LoadMetadataDetails[] listOfLoadFolderDetailsArray =
SegmentStatusManager.readLoadMetadata(metaDataFilepath);
-
+ Boolean isUpdateRequired = false;
Review comment:
use boolean instead of Boolean
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
