Caideyipi commented on code in PR #17664:
URL: https://github.com/apache/iotdb/pull/17664#discussion_r3295886278
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/read/filescan/impl/UnclosedFileScanHandleImpl.java:
##########
@@ -48,6 +51,8 @@ public class UnclosedFileScanHandleImpl implements
IFileScanHandle {
private final TsFileResource tsFileResource;
private final Map<IDeviceID, Map<String, List<IChunkMetadata>>>
deviceToChunkMetadataMap;
private final Map<IDeviceID, Map<String, List<IChunkHandle>>>
deviceToMemChunkHandleMap;
+ private final Map<IDeviceID, List<TimeRange>> deviceToDeletionRanges;
+ private final Map<IDeviceID, Map<String, List<TimeRange>>>
deviceToTimeSeriesDeletionRanges;
Review Comment:
Updated. Both lazy caches are now ConcurrentHashMaps and use
computeIfAbsent. appendDeletionRanges now copies each TimeRange before merge,
so sortAndMerge cannot mutate intervals owned by chunk metadata.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/TsFileManager.java:
##########
@@ -223,10 +216,18 @@ public void removeAll(List<TsFileResource>
tsFileResourceList, boolean sequence)
writeLock("removeAll");
try {
for (TsFileResource resource : tsFileResourceList) {
- remove(resource, sequence);
+ removeFromPartitionFileList(resource, sequence);
}
} finally {
- writeLock("removeAll");
+ writeUnlock();
+ }
Review Comment:
Updated the PR description to call out the removeAll write-lock leak fix
explicitly.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/TsFileManager.java:
##########
@@ -223,10 +216,18 @@ public void removeAll(List<TsFileResource>
tsFileResourceList, boolean sequence)
writeLock("removeAll");
try {
for (TsFileResource resource : tsFileResourceList) {
- remove(resource, sequence);
+ removeFromPartitionFileList(resource, sequence);
}
} finally {
- writeLock("removeAll");
+ writeUnlock();
+ }
+ }
+
+ private void removeFromPartitionFileList(TsFileResource tsFileResource,
boolean sequence) {
+ Map<Long, TsFileResourceList> selectedMap = sequence ? sequenceFiles :
unsequenceFiles;
+ TsFileResourceList tsFileResources =
selectedMap.get(tsFileResource.getTimePartition());
+ if (tsFileResources != null && tsFileResources.remove(tsFileResource)) {
Review Comment:
Confirmed. TsFileManager inserts resources into partition maps using the
same resource.getTimePartition() key; managed resources are not moved to
another partition in place. Replacement and rename flows build or update the
resource before adding or replacing it in the manager.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/ModificationUtils.java:
##########
@@ -82,7 +82,11 @@ public static void modifyChunkMetaData(
if (range.contains(metaData.getStartTime(),
metaData.getEndTime())) {
return true;
} else {
- if (range.overlaps(new TimeRange(metaData.getStartTime(),
metaData.getEndTime()))) {
+ if (overlap(
Review Comment:
Confirmed. ModificationUtils.overlap uses closed-interval semantics: endB >=
startA && startB <= endA, matching TimeRange.overlaps inclusive endpoints.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java:
##########
@@ -572,40 +558,30 @@ private static void fillRemainingBitMap(
}
private static void fillTimeColumn(
- int rowCount, ByteArrayOutputStream[] byteArrayOutputStreams,
TSQueryDataSet tsQueryDataSet) {
- // calculate the time buffer size
- int timeOccupation = rowCount * 8;
- ByteBuffer timeBuffer = ByteBuffer.allocate(timeOccupation);
- timeBuffer.put(byteArrayOutputStreams[0].toByteArray());
- timeBuffer.flip();
- tsQueryDataSet.setTime(timeBuffer);
+ int rowCount, PublicBAOS[] byteArrayOutputStreams, TSQueryDataSet
tsQueryDataSet) {
Review Comment:
Done. Dropped the unused rowCount parameters and removed valueOccupation
entirely; columnNum now derives from byteArrayOutputStreams.length / 2.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java:
##########
@@ -572,40 +558,30 @@ private static void fillRemainingBitMap(
}
private static void fillTimeColumn(
- int rowCount, ByteArrayOutputStream[] byteArrayOutputStreams,
TSQueryDataSet tsQueryDataSet) {
- // calculate the time buffer size
- int timeOccupation = rowCount * 8;
- ByteBuffer timeBuffer = ByteBuffer.allocate(timeOccupation);
- timeBuffer.put(byteArrayOutputStreams[0].toByteArray());
- timeBuffer.flip();
- tsQueryDataSet.setTime(timeBuffer);
+ int rowCount, PublicBAOS[] byteArrayOutputStreams, TSQueryDataSet
tsQueryDataSet) {
+ tsQueryDataSet.setTime(wrapBuffer(byteArrayOutputStreams[0]));
}
private static void fillValueColumnsAndBitMaps(
int rowCount,
- ByteArrayOutputStream[] byteArrayOutputStreams,
+ PublicBAOS[] byteArrayOutputStreams,
int[] valueOccupation,
TSQueryDataSet tsQueryDataSet) {
- // calculate the bitmap buffer size
- int bitmapOccupation = (rowCount + 7) / 8;
-
- List<ByteBuffer> bitmapList = new LinkedList<>();
- List<ByteBuffer> valueList = new LinkedList<>();
+ int columnNum = valueOccupation.length;
+ List<ByteBuffer> bitmapList = new ArrayList<>(columnNum);
+ List<ByteBuffer> valueList = new ArrayList<>(columnNum);
for (int i = 1; i < byteArrayOutputStreams.length; i += 2) {
- ByteBuffer valueBuffer = ByteBuffer.allocate(valueOccupation[(i - 1) /
2]);
- valueBuffer.put(byteArrayOutputStreams[i].toByteArray());
- valueBuffer.flip();
- valueList.add(valueBuffer);
-
- ByteBuffer bitmapBuffer = ByteBuffer.allocate(bitmapOccupation);
- bitmapBuffer.put(byteArrayOutputStreams[i + 1].toByteArray());
- bitmapBuffer.flip();
- bitmapList.add(bitmapBuffer);
+ valueList.add(wrapBuffer(byteArrayOutputStreams[i]));
+ bitmapList.add(wrapBuffer(byteArrayOutputStreams[i + 1]));
}
tsQueryDataSet.setBitmapList(bitmapList);
tsQueryDataSet.setValueList(valueList);
}
+ private static ByteBuffer wrapBuffer(PublicBAOS outputStream) {
Review Comment:
Confirmed. wrapBuffer is only called after serialization is complete and
immediately before populating TSQueryDataSet; no writes happen to those
PublicBAOS/DataOutputStream instances after wrapping.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
