Skip to content

Conversation

@shuwenwei
Copy link
Member

@shuwenwei shuwenwei commented Jun 17, 2025

Description

Merge LastQueryScanNode of same device
For a scenario with 31185 non-aligned devices and 1 measurement points per device.
When the last cache is hit, the optimization effect of executing the following SQL statement is:

select last value from root.reactor.**

Before this optimization:

img_v3_02np_75b07e13-a474-47d6-b5b1-dcf837e07f9g After this optimization: 截屏2025-07-02 10 46 17

For a scenario with 2 * 2 * 4 non-aligned devices and 80,000 measurement points per device.
When the last cache is hit, the optimization effect of executing the following SQL statement is:

select last * from root.db.aaaaaaa0.bbbbbbb0.**

Before this optimization:
image
After this optimization:
截屏2025-06-18 12 00 25

@codecov
Copy link

codecov bot commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 59.71223% with 168 lines in your changes missing coverage. Please review.

Project coverage is 39.11%. Comparing base (b8fd089) to head (352191f).
Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
...ueryengine/plan/planner/OperatorTreeGenerator.java 0.00% 45 Missing ⚠️
...an/planner/plan/node/source/LastQueryScanNode.java 52.94% 32 Missing ⚠️
.../planner/plan/node/process/last/LastQueryNode.java 55.22% 30 Missing ⚠️
.../process/last/AbstractUpdateLastCacheOperator.java 13.33% 13 Missing ⚠️
...db/db/queryengine/plan/analyze/AnalyzeVisitor.java 70.58% 10 Missing ⚠️
...b/queryengine/plan/planner/LogicalPlanBuilder.java 74.35% 10 Missing ⚠️
...gine/plan/planner/distribution/SourceRewriter.java 88.88% 7 Missing ⚠️
...ngine/execution/fragment/DataNodeQueryContext.java 16.66% 5 Missing ⚠️
...er/distribution/SimpleFragmentParallelPlanner.java 16.66% 5 Missing ⚠️
...e/iotdb/db/queryengine/common/MPPQueryContext.java 81.81% 2 Missing ⚠️
... and 6 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15735      +/-   ##
============================================
+ Coverage     39.09%   39.11%   +0.01%     
  Complexity      198      198              
============================================
  Files          4839     4839              
  Lines        314856   315188     +332     
  Branches      39530    39567      +37     
============================================
+ Hits         123108   123300     +192     
- Misses       191748   191888     +140     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines 161 to 185
public List<TRegionReplicaSet> getDataRegionReplicaSetWithTimeFilter(
String db, TSeriesPartitionSlot tSeriesPartitionSlot, Filter timeFilter) {
Map<TTimePartitionSlot, List<TRegionReplicaSet>> regionReplicaSetMap =
dataPartitionMap
.getOrDefault(db, Collections.emptyMap())
.getOrDefault(tSeriesPartitionSlot, Collections.emptyMap());
if (regionReplicaSetMap.isEmpty()) {
return Collections.singletonList(NOT_ASSIGNED);
}
List<TRegionReplicaSet> replicaSets = new ArrayList<>();
Set<TRegionReplicaSet> uniqueValues = new HashSet<>();
for (Entry<TTimePartitionSlot, List<TRegionReplicaSet>> entry :
regionReplicaSetMap.entrySet()) {
if (!TimePartitionUtils.satisfyPartitionStartTime(timeFilter, entry.getKey().startTime)) {
continue;
}
for (TRegionReplicaSet tRegionReplicaSet : entry.getValue()) {
if (uniqueValues.add(tRegionReplicaSet)) {
replicaSets.add(tRegionReplicaSet);
}
}
}
return replicaSets;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never used

new LastQueryOperator(
driverContext.getOperatorContexts().get(4),
ImmutableList.of(updateLastCacheOperator1, updateLastCacheOperator2),
Arrays.asList(updateLastCacheOperator1, updateLastCacheOperator2),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need to change it into a mutable list?

return null;
} else if (!tsBlock.isEmpty()) {
LastQueryUtil.appendLastValue(tsBlockBuilder, tsBlock);
continue;
Copy link
Contributor

@JackieTien97 JackieTien97 Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

child.next can only be called once in parent's next call

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return null;

previousTsBlockIndex = 0;
if (previousTsBlock == null) {
return true;
if (previousTsBlock == null || children.get(currentIndex).hasNextWithTimer()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should only call hasNextWithTimer once

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just return

// if children contains LastTransformNode, this variable is only used in distribute plan
private boolean containsLastTransformNode;

private Map<IMeasurementSchema, Integer> measurementSchema2IdxMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comments about this field to describe its life cycle, like when will this field be set to null

Comment on lines 148 to 156
@Override
public List<PlanNode> getChildren() {
return children;
public void serialize(DataOutputStream stream) throws IOException {
super.serialize(stream);
}

@Override
public void addChild(PlanNode child) {
children.add(child);
public void serialize(ByteBuffer byteBuffer) {
super.serialize(byteBuffer);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to override it?

shuwenwei and others added 8 commits July 1, 2025 16:29
…ine/plan/analyze/AnalyzeVisitor.java

Co-authored-by: Jackie Tien <[email protected]>
…ine/plan/planner/plan/node/process/last/LastQueryNode.java

Co-authored-by: Jackie Tien <[email protected]>
…ine/plan/planner/plan/node/process/last/LastQueryNode.java

Co-authored-by: Jackie Tien <[email protected]>
…ine/plan/planner/plan/node/process/last/LastQueryNode.java

Co-authored-by: Jackie Tien <[email protected]>
…ine/plan/planner/plan/node/source/LastQueryScanNode.java

Co-authored-by: Jackie Tien <[email protected]>
…ine/plan/planner/plan/node/process/last/LastQueryNode.java

Co-authored-by: Jackie Tien <[email protected]>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 1, 2025

@JackieTien97 JackieTien97 merged commit d86b86e into master Jul 2, 2025
60 of 66 checks passed
@JackieTien97 JackieTien97 deleted the mergeLastQueryScanNodeOfSameDevice branch July 2, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants