jt2594838 commented on a change in pull request #1400:
URL: https://github.com/apache/incubator-iotdb/pull/1400#discussion_r448861552
##########
File path:
server/src/test/java/org/apache/iotdb/db/engine/merge/MergeTaskTest.java
##########
@@ -218,7 +218,7 @@ public void testPartialMerge3() throws Exception {
public void mergeWithDeletionTest() throws Exception {
try {
seqResources.get(0).getModFile().write(new Deletion(new
Path(deviceIds[0],
- measurementSchemas[0].getMeasurementId()), 10000, 49));
+ measurementSchemas[0].getMeasurementId()), 10000, 0,49));
Review comment:
Please mind the format.
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryUtils.java
##########
@@ -45,46 +49,52 @@ private QueryUtils() {
*/
public static void modifyChunkMetaData(List<ChunkMetadata> chunkMetaData,
List<Modification> modifications) {
- int modIndex = 0;
+ List<Modification> sortedModifications = sortModifications(modifications);
for (int metaIndex = 0; metaIndex < chunkMetaData.size(); metaIndex++) {
ChunkMetadata metaData = chunkMetaData.get(metaIndex);
- for (int j = modIndex; j < modifications.size(); j++) {
- // iterate each modification to find the max deletion time
- Modification modification = modifications.get(j);
+ for (Modification modification : sortedModifications) {
if (modification.getVersionNum() > metaData.getVersion()) {
- // this modification is after the Chunk, try modifying the chunk
- // if this modification succeeds, update modIndex so in the next
loop the previous
- // modifications will not be examined
- modIndex = doModifyChunkMetaData(modification, metaData)? j :
modIndex;
- } else {
- // skip old modifications for next metadata
- modIndex++;
+ doModifyChunkMetaData(modification, metaData);
}
}
}
// remove chunks that are completely deleted
chunkMetaData.removeIf(metaData -> {
- if (metaData.getDeletedAt() >= metaData.getEndTime()) {
- return true;
- } else {
- if (metaData.getDeletedAt() >= metaData.getStartTime()) {
- metaData.setModified(true);
+ long lower = metaData.getStartTime();
+ long upper = metaData.getEndTime();
+ if (metaData.getDeleteIntervalList() != null) {
+ for (Pair<Long, Long> range : metaData.getDeleteIntervalList()) {
+ if (upper < range.left) {
+ break;
+ }
+ if (range.left <= lower && lower <= range.right) {
+ metaData.setModified(true);
+ if (upper <= range.right) {
+ return true;
+ }
+ lower = range.right;
+ } else if (lower < range.left) {
+ metaData.setModified(true);
+ break;
+ }
}
- return false;
}
+ return false;
});
}
- private static boolean doModifyChunkMetaData(Modification modification,
ChunkMetadata metaData) {
+ private static LinkedList<Modification> sortModifications(List<Modification>
modifications) {
+ return modifications.stream().filter(x -> x instanceof Deletion)
+ .sorted(Comparator.comparingLong(mods -> ((Deletion)
mods).getStartTime()))
+ .collect(Collectors.toCollection(LinkedList::new));
+ }
Review comment:
I wonder why LinkedList is preferred.
##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -747,14 +747,37 @@ public void deleteData(String path, long time)
* delete data <= time in multiple timeseries
*
* @param paths data in which time series to delete
- * @param time data with time stamp less than or equal to time will be
deleted
+ * @param endTime data with time stamp less than or equal to time will be
deleted
*/
- public void deleteData(List<String> paths, long time)
+ public void deleteData(List<String> paths, long endTime)
throws IoTDBConnectionException, StatementExecutionException {
TSDeleteDataReq request = new TSDeleteDataReq();
request.setSessionId(sessionId);
request.setPaths(paths);
- request.setTimestamp(time);
+ request.setStartTime(Long.MIN_VALUE);
+ request.setEndTime(endTime);
+
Review comment:
It may simply be changed to a call of `deleteData(paths, Long.MIN_VALUE,
endtime)`.
##########
File path:
server/src/main/java/org/apache/iotdb/db/utils/datastructure/TVList.java
##########
@@ -531,6 +530,17 @@ public boolean hasNextTimeValuePair() {
return hasCachedPair;
}
+ private boolean isPointDeleted(long timestamp) {
+ if (deletionList != null) {
+ for (Pair<Long, Long> del : deletionList) {
+ if (del.left <= timestamp && timestamp <= del.right) {
+ return true;
+ }
+ }
Review comment:
I think it is not necessary to start from the first of the deletions at
each call of `isPointDeleted()`, as this method is called with ordered
timestamps, if one timestamp is after the range of one deletion, all remaining
timestamps will either.
----------------------------------------------------------------
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:
us...@infra.apache.org
