You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "mymeiyi (via GitHub)" <gi...@apache.org> on 2026/02/05 10:47:45 UTC
[PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
mymeiyi opened a new pull request, #60543:
URL: https://github.com/apache/doris/pull/60543
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4074410443
# FE UT Coverage Report
Increment line coverage ` 13.30% (31/233)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_72cf29d664b5bbc35c649177f43639ac08ee8732/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_72cf29d664b5bbc35c649177f43639ac08ee8732/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789393147
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -365,4 +507,77 @@ private GetTabletStatsResponse getTabletStats(GetTabletStatsRequest request)
public List<OlapTable.Statistics> getCloudTableStats() {
return this.cloudTableStatsList;
}
+
+ public void addActiveTablets(List<Long> tabletIds) {
+ if (tabletIds == null || tabletIds.isEmpty()) {
+ return;
+ }
+ synchronized (activeTablets) {
+ activeTablets.addAll(tabletIds);
+ }
+ }
+
+ // master FE send update tablet stats rpc to other FEs
+ private void pushTabletStats(GetTabletStatsResponse response) {
+ List<Frontend> frontends = getFrontends();
+ if (frontends == null || frontends.isEmpty()) {
Review Comment:
does the check imply current caller is "master"?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2768410662
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4367,16 +4371,34 @@ public TStatus reportCommitTxnResult(TReportCommitTxnResultRequest request) thro
return new TStatus(TStatusCode.NOT_MASTER);
}
- LOG.info("receive load stats report request: {}, backend: {}, dbId: {}, txnId: {}, label: {}",
- request, clientAddr, request.getDbId(), request.getTxnId(), request.getLabel());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("receive load stats report from backend: {}, dbId: {}, txnId: {}, label: {}, tabletIds: {}",
+ clientAddr, request.getDbId(), request.getTxnId(), request.getLabel(), request.getTabletIds());
+ }
try {
- byte[] receivedProtobufBytes = request.getPayload();
- if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
- return new TStatus(TStatusCode.INVALID_ARGUMENT);
+ if (request.isSetTxnId() && request.getTxnId() != -1) {
+ byte[] receivedProtobufBytes = request.getPayload();
+ if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
+ return new TStatus(TStatusCode.INVALID_ARGUMENT);
+ }
+ CommitTxnResponse commitTxnResponse = CommitTxnResponse.parseFrom(receivedProtobufBytes);
+ Env.getCurrentGlobalTransactionMgr().afterCommitTxnResp(commitTxnResponse, request.getTabletIds());
Review Comment:
Potential NullPointerException: `request.getTabletIds()` can be null if the optional field is not set in the Thrift request. Should add a null check using `request.isSetTabletIds()` before accessing the list, or provide a default empty list value.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -365,4 +510,70 @@ private GetTabletStatsResponse getTabletStats(GetTabletStatsRequest request)
public List<OlapTable.Statistics> getCloudTableStats() {
return this.cloudTableStatsList;
}
+
+ public void addActiveTablets(List<Long> tabletIds) {
+ synchronized (activeTablets) {
+ activeTablets.addAll(tabletIds);
+ }
+ }
Review Comment:
Potential NullPointerException: The `tabletIds` parameter is not checked for null before calling `addAll`. If a null list is passed in, this will throw a NullPointerException. Should add a null check before synchronizing and adding to the set.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -541,6 +543,12 @@ public void afterCommitTxnResp(CommitTxnResponse commitTxnResponse) {
if (sb.length() > 0) {
LOG.info("notify partition first load. {}", sb);
}
+ // 4. notify update tablet stats
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("force sync tablet stats for txnId: {}, tabletNum: {}, tabletIds: {}", txnId,
+ tabletIds.size(), tabletIds);
+ }
+ ((CloudTabletStatMgr) (env.getTabletStatMgr())).addActiveTablets(tabletIds);
Review Comment:
Potential ClassCastException: There's no check to verify that `env.getTabletStatMgr()` is actually an instance of `CloudTabletStatMgr` before casting. Although this is in CloudGlobalTransactionMgr (which should only run in cloud mode), adding an instanceof check would make the code more robust against configuration errors.
```suggestion
if (env.getTabletStatMgr() instanceof CloudTabletStatMgr) {
((CloudTabletStatMgr) env.getTabletStatMgr()).addActiveTablets(tabletIds);
} else {
LOG.warn("tabletStatMgr is not instance of CloudTabletStatMgr, skip updating active tablets for txnId: {}",
txnId);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -655,11 +663,12 @@ private void commitTransactionWithoutLock(long dbId, List<Table> tableList, long
}
final CommitTxnRequest commitTxnRequest = builder.build();
- executeCommitTxnRequest(commitTxnRequest, transactionId, is2PC, txnCommitAttachment);
+ executeCommitTxnRequest(commitTxnRequest, transactionId, is2PC, txnCommitAttachment,
+ tabletCommitInfos.stream().map(t -> t.getTabletId()).collect(Collectors.toList()));
Review Comment:
Potential NullPointerException: `tabletCommitInfos` can be null (as seen in line 1740 where it's passed as null). Calling `tabletCommitInfos.stream()` on line 667 will throw a NullPointerException. Should add a null check and handle the case where tabletCommitInfos is null, potentially passing an empty list to the method.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -365,4 +510,70 @@ private GetTabletStatsResponse getTabletStats(GetTabletStatsRequest request)
public List<OlapTable.Statistics> getCloudTableStats() {
return this.cloudTableStatsList;
}
+
+ public void addActiveTablets(List<Long> tabletIds) {
+ synchronized (activeTablets) {
+ activeTablets.addAll(tabletIds);
+ }
+ }
+
+ // master FE send update tablet stats rpc to other FEs
+ private void pushTabletStats(GetTabletStatsResponse response) {
+ List<Frontend> frontends = getFrontends();
+ if (frontends == null || frontends.isEmpty()) {
+ return;
+ }
+ TSyncCloudTabletStatsRequest request = new TSyncCloudTabletStatsRequest();
+ request.setTabletStatsPb(ByteBuffer.wrap(response.toByteArray()));
+ for (Frontend fe : frontends) {
+ SYNC_TABLET_STATS_THREAD_POOL.submit(() -> {
+ try {
+ pushTabletStatsToFe(request, fe);
+ } catch (Exception e) {
+ LOG.warn("push tablet stats to fe: {} error", fe.getHost(), e);
+ }
+ });
+ }
+ }
+
+ private void pushTabletStatsToFe(TSyncCloudTabletStatsRequest request, Frontend fe) {
+ FrontendService.Client client = null;
+ TNetworkAddress addr = new TNetworkAddress(fe.getHost(), fe.getRpcPort());
+ boolean ok = false;
+ try {
+ client = ClientPool.frontendStatsPool.borrowObject(addr);
+ TSyncCloudTabletStatsResult result = client.syncCloudTabletStats(request);
+ ok = true;
+ if (result.getStatus().getStatusCode() != TStatusCode.OK) {
+ LOG.warn("failed to update cloud tablet stats to frontend {}:{}, err: {}", fe.getHost(),
+ fe.getRpcPort(), result.getStatus().getErrorMsgs());
+ }
+ } catch (Exception e) {
+ LOG.warn("failed to update update cloud tablet stats to frontend {}:{}", fe.getHost(), fe.getRpcPort(), e);
+ } finally {
+ if (ok) {
+ ClientPool.frontendStatsPool.returnObject(addr, client);
+ } else {
+ ClientPool.frontendStatsPool.invalidateObject(addr, client);
Review Comment:
Potential resource leak: If `ClientPool.frontendStatsPool.borrowObject(addr)` throws an exception, the client will be null, but `invalidateObject` will still be called with a null client. While this may be handled internally by the pool, it would be cleaner to check if client is not null before calling invalidateObject.
```suggestion
if (client != null) {
if (ok) {
ClientPool.frontendStatsPool.returnObject(addr, client);
} else {
ClientPool.frontendStatsPool.invalidateObject(addr, client);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/TabletsProcDir.java:
##########
@@ -194,6 +207,11 @@ public List<List<Comparable>> fetchComparableResult(long version, long backendId
} finally {
table.readUnlock();
}
+ if (tabletIds != null && !tabletIds.isEmpty()) {
+ LOG.info("force sync tablet stats for table: {}, tabletNum: {}, tabletIds: {}", table,
+ tabletIds.size(), tabletIds);
+ ((CloudTabletStatMgr) (Env.getCurrentEnv().getTabletStatMgr())).addActiveTablets(tabletIds);
Review Comment:
Potential ClassCastException: There's no check to verify that `Env.getCurrentEnv().getTabletStatMgr()` is actually an instance of `CloudTabletStatMgr` before casting. Although this code path checks `Config.isCloudMode()`, adding an instanceof check would make the code more defensive. Additionally, the result of the cast is not checked for null before calling methods on it.
```suggestion
Object tabletStatMgr = Env.getCurrentEnv().getTabletStatMgr();
if (tabletStatMgr instanceof CloudTabletStatMgr) {
((CloudTabletStatMgr) tabletStatMgr).addActiveTablets(tabletIds);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4707,6 +4729,37 @@ public TGetOlapTableMetaResult getOlapTableMeta(TGetOlapTableMetaRequest request
}
}
+ @Override
+ public TSyncCloudTabletStatsResult syncCloudTabletStats(TSyncCloudTabletStatsRequest request)
+ throws TException {
+ TSyncCloudTabletStatsResult response = new TSyncCloudTabletStatsResult();
+ TStatus status = new TStatus(TStatusCode.OK);
+ response.setStatus(status);
+
+ if (Env.getCurrentEnv().isMaster()) {
+ LOG.warn("syncCloudTabletStats called on master, ignoring");
+ return response;
+ }
+
+ byte[] receivedProtobufBytes = request.getTabletStatsPb();
+ if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
+ status.setStatusCode(TStatusCode.INVALID_ARGUMENT);
+ status.addToErrorMsgs("TabletStatsPb is null or empty");
+ return response;
+ }
+ GetTabletStatsResponse getTabletStatsResponse;
+ try {
+ getTabletStatsResponse = GetTabletStatsResponse.parseFrom(receivedProtobufBytes);
+ } catch (Exception e) {
+ status.setStatusCode(TStatusCode.INVALID_ARGUMENT);
+ status.addToErrorMsgs("parse GetTabletStatsResponse error: " + e.getMessage());
+ return response;
+ }
+ CloudTabletStatMgr cloudTabletStatMgr = (CloudTabletStatMgr) (Env.getCurrentEnv().getTabletStatMgr());
+ cloudTabletStatMgr.syncTabletStats(getTabletStatsResponse);
Review Comment:
Missing null check: After casting, there's no null check for `cloudTabletStatMgr` before calling `syncTabletStats`. If `getTabletStatMgr()` returns null, this will throw a NullPointerException. Should verify that `cloudTabletStatMgr` is not null before invoking methods on it.
```suggestion
if (cloudTabletStatMgr != null) {
cloudTabletStatMgr.syncTabletStats(getTabletStatsResponse);
} else {
LOG.warn("TabletStatMgr is null in syncCloudTabletStats, skipping sync");
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -333,24 +458,44 @@ private void updateStatInfo(List<Long> dbIds) {
(System.currentTimeMillis() - start));
}
- private void updateTabletStat(GetTabletStatsResponse response) {
+ private void updateTabletStat(GetTabletStatsResponse response, boolean activeUpdate) {
TabletInvertedIndex invertedIndex = Env.getCurrentInvertedIndex();
for (TabletStatsPB stat : response.getTabletStatsList()) {
List<Replica> replicas = invertedIndex.getReplicasByTabletId(stat.getIdx().getTabletId());
if (replicas == null || replicas.isEmpty() || replicas.get(0) == null) {
continue;
}
Replica replica = replicas.get(0);
+ boolean statsChanged = replica.getDataSize() != stat.getDataSize()
+ || replica.getRowsetCount() != stat.getNumRowsets()
+ || replica.getSegmentCount() != stat.getNumSegments()
+ || replica.getRowCount() != stat.getNumRows()
+ || replica.getLocalInvertedIndexSize() != stat.getIndexSize()
+ || replica.getLocalSegmentSize() != stat.getSegmentSize();
replica.setDataSize(stat.getDataSize());
replica.setRowsetCount(stat.getNumRowsets());
replica.setSegmentCount(stat.getNumSegments());
replica.setRowCount(stat.getNumRows());
replica.setLocalInvertedIndexSize(stat.getIndexSize());
replica.setLocalSegmentSize(stat.getSegmentSize());
+
Review Comment:
Potential ClassCastException: The code casts `replica` to `CloudReplica` without verifying it's actually an instance of CloudReplica. While this method should only be called in cloud mode, adding an instanceof check would make the code more defensive and prevent unexpected ClassCastExceptions.
```suggestion
if (!(replica instanceof CloudReplica)) {
continue;
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/alter/CloudSchemaChangeJobV2.java:
##########
@@ -91,6 +92,15 @@ protected void commitShadowIndex() throws AlterCancelException {
}
LOG.info("commitShadowIndex finished, dbId:{}, tableId:{}, jobId:{}, shadowIdxList:{}",
dbId, tableId, jobId, shadowIdxList);
+
+ List<Long> tabletIds = partitionIndexMap.cellSet().stream()
+ .flatMap(cell -> cell.getValue().getTablets().stream().map(Tablet::getId))
+ .collect(Collectors.toList());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("force sync tablet stats for table: {}, tabletNum: {}, tabletIds: {}", tableId,
+ tabletIds.size(), tabletIds);
+ }
+ ((CloudTabletStatMgr) (Env.getCurrentEnv().getTabletStatMgr())).addActiveTablets(tabletIds);
Review Comment:
Potential ClassCastException: There's no check to verify that `Env.getCurrentEnv().getTabletStatMgr()` is actually an instance of `CloudTabletStatMgr` before casting. Although this is in CloudSchemaChangeJobV2 (which should only run in cloud mode), adding an instanceof check would make the code more robust.
```suggestion
Object tabletStatMgr = Env.getCurrentEnv().getTabletStatMgr();
if (tabletStatMgr instanceof CloudTabletStatMgr) {
((CloudTabletStatMgr) tabletStatMgr).addActiveTablets(tabletIds);
} else {
LOG.warn("tabletStatMgr is not instance of CloudTabletStatMgr when committing shadow index for "
+ "tableId: {}, jobId: {}. Actual type: {}", tableId, jobId,
(tabletStatMgr != null ? tabletStatMgr.getClass().getName() : "null"));
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -60,11 +101,42 @@ public CloudTabletStatMgr() {
@Override
protected void runAfterCatalogReady() {
LOG.info("cloud tablet stat begin");
- List<Long> dbIds = getAllTabletStats();
+ // get stats for active tablets
+ Set<Long> copiedTablets;
+ synchronized (activeTablets) {
+ copiedTablets = new HashSet<>(activeTablets);
+ activeTablets.clear();
+ }
+ getActiveTabletStats(copiedTablets);
+
+ // get stats by interval
+ List<Long> dbIds = getAllTabletStats(cloudTablet -> {
+ if (copiedTablets.contains(cloudTablet.getId())) {
+ return false;
+ }
+ List<Replica> replicas = Env.getCurrentInvertedIndex().getReplicas(cloudTablet.getId());
+ if (replicas == null || replicas.isEmpty()) {
+ return false;
+ }
+ CloudReplica cloudReplica = (CloudReplica) replicas.get(0);
Review Comment:
Potential ClassCastException: The code casts `replicas.get(0)` to `CloudReplica` without verifying it's actually an instance of CloudReplica. While this should be safe in cloud mode, adding an instanceof check would make the code more defensive against unexpected states.
```suggestion
Replica firstReplica = replicas.get(0);
if (!(firstReplica instanceof CloudReplica)) {
LOG.warn("unexpected replica type for tabletId {}: {}", cloudTablet.getId(),
firstReplica != null ? firstReplica.getClass().getName() : "null");
return false;
}
CloudReplica cloudReplica = (CloudReplica) firstReplica;
```
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -541,6 +543,12 @@ public void afterCommitTxnResp(CommitTxnResponse commitTxnResponse) {
if (sb.length() > 0) {
LOG.info("notify partition first load. {}", sb);
}
+ // 4. notify update tablet stats
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("force sync tablet stats for txnId: {}, tabletNum: {}, tabletIds: {}", txnId,
+ tabletIds.size(), tabletIds);
+ }
+ ((CloudTabletStatMgr) (env.getTabletStatMgr())).addActiveTablets(tabletIds);
Review Comment:
Potential NullPointerException: `tabletIds` parameter is not checked for null before calling `.size()` in the debug log. If a caller passes null (which appears possible from the interface signature), this will throw an NPE. Should add a null check before accessing the list.
```suggestion
tabletIds == null ? 0 : tabletIds.size(), tabletIds);
}
if (tabletIds != null && !tabletIds.isEmpty()) {
((CloudTabletStatMgr) (env.getTabletStatMgr())).addActiveTablets(tabletIds);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -87,17 +159,23 @@ private List<Long> getAllTabletStats() {
OlapTable tbl = (OlapTable) table;
for (Partition partition : tbl.getAllPartitions()) {
for (MaterializedIndex index : partition.getMaterializedIndices(IndexExtState.VISIBLE)) {
- for (Long tabletId : index.getTabletIdsInOrder()) {
+ for (Tablet tablet : index.getTablets()) {
+ if (predicate != null) {
Review Comment:
Potential ClassCastException: The code casts `tablet` to `CloudTablet` without verifying it's actually an instance of CloudTablet. While this should be safe in cloud mode, adding an instanceof check would make the code more defensive against unexpected states.
```suggestion
if (predicate != null) {
if (!(tablet instanceof CloudTablet)) {
continue;
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4367,16 +4371,34 @@ public TStatus reportCommitTxnResult(TReportCommitTxnResultRequest request) thro
return new TStatus(TStatusCode.NOT_MASTER);
}
- LOG.info("receive load stats report request: {}, backend: {}, dbId: {}, txnId: {}, label: {}",
- request, clientAddr, request.getDbId(), request.getTxnId(), request.getLabel());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("receive load stats report from backend: {}, dbId: {}, txnId: {}, label: {}, tabletIds: {}",
+ clientAddr, request.getDbId(), request.getTxnId(), request.getLabel(), request.getTabletIds());
+ }
try {
- byte[] receivedProtobufBytes = request.getPayload();
- if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
- return new TStatus(TStatusCode.INVALID_ARGUMENT);
+ if (request.isSetTxnId() && request.getTxnId() != -1) {
+ byte[] receivedProtobufBytes = request.getPayload();
+ if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
+ return new TStatus(TStatusCode.INVALID_ARGUMENT);
+ }
+ CommitTxnResponse commitTxnResponse = CommitTxnResponse.parseFrom(receivedProtobufBytes);
+ Env.getCurrentGlobalTransactionMgr().afterCommitTxnResp(commitTxnResponse, request.getTabletIds());
+ } else {
+ // compaction notify update tablet stats
+ CloudTabletStatMgr cloudTabletStatMgr = (CloudTabletStatMgr) (Env.getCurrentEnv().getTabletStatMgr());
+ if (cloudTabletStatMgr == null) {
+ LOG.warn("CloudTabletStatMgr is null");
+ TStatus status = new TStatus(TStatusCode.INTERNAL_ERROR);
+ status.addToErrorMsgs("CloudTabletStatMgr is null");
+ return status;
+ }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("force sync tablet stats for txnId: {}, tabletNum: {}, tabletIds: {}", request.txnId,
+ request.getTabletIds().size(), request.getTabletIds());
+ }
+ cloudTabletStatMgr.addActiveTablets(request.getTabletIds());
Review Comment:
Potential NullPointerException: `request.getTabletIds()` can be null if the field is not set in the Thrift request. This will cause an NPE when calling `.size()` on line 4398 and when passing it to `addActiveTablets`. Should check if `request.isSetTabletIds()` and handle the null case before accessing the list.
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4707,6 +4729,37 @@ public TGetOlapTableMetaResult getOlapTableMeta(TGetOlapTableMetaRequest request
}
}
+ @Override
+ public TSyncCloudTabletStatsResult syncCloudTabletStats(TSyncCloudTabletStatsRequest request)
+ throws TException {
+ TSyncCloudTabletStatsResult response = new TSyncCloudTabletStatsResult();
+ TStatus status = new TStatus(TStatusCode.OK);
+ response.setStatus(status);
+
+ if (Env.getCurrentEnv().isMaster()) {
+ LOG.warn("syncCloudTabletStats called on master, ignoring");
+ return response;
+ }
+
+ byte[] receivedProtobufBytes = request.getTabletStatsPb();
+ if (receivedProtobufBytes == null || receivedProtobufBytes.length <= 0) {
+ status.setStatusCode(TStatusCode.INVALID_ARGUMENT);
+ status.addToErrorMsgs("TabletStatsPb is null or empty");
+ return response;
+ }
+ GetTabletStatsResponse getTabletStatsResponse;
+ try {
+ getTabletStatsResponse = GetTabletStatsResponse.parseFrom(receivedProtobufBytes);
+ } catch (Exception e) {
+ status.setStatusCode(TStatusCode.INVALID_ARGUMENT);
+ status.addToErrorMsgs("parse GetTabletStatsResponse error: " + e.getMessage());
+ return response;
+ }
+ CloudTabletStatMgr cloudTabletStatMgr = (CloudTabletStatMgr) (Env.getCurrentEnv().getTabletStatMgr());
Review Comment:
Potential ClassCastException: There's no check to verify that `Env.getCurrentEnv().getTabletStatMgr()` is actually an instance of `CloudTabletStatMgr` before casting. In cloud mode, this is likely safe, but in non-cloud mode or during transitions, this could throw a ClassCastException. Consider adding an `instanceof` check or wrapping in a try-catch for safety.
```suggestion
if (!(Env.getCurrentEnv().getTabletStatMgr() instanceof CloudTabletStatMgr)) {
status.setStatusCode(TStatusCode.INTERNAL_ERROR);
status.addToErrorMsgs("TabletStatMgr is not an instance of CloudTabletStatMgr");
return response;
}
CloudTabletStatMgr cloudTabletStatMgr = (CloudTabletStatMgr) Env.getCurrentEnv().getTabletStatMgr();
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3871818847
# BE UT Coverage Report
Increment line coverage `0.00% (0/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/631284d5b22f079e4928cbab8f35eea4c670ef78_631284d5b22f079e4928cbab8f35eea4c670ef78/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/631284d5b22f079e4928cbab8f35eea4c670ef78_631284d5b22f079e4928cbab8f35eea4c670ef78/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.77% (19463/36882) |
| Line Coverage | 36.28% (181349/499913) |
| Region Coverage | 32.63% (140699/431180) |
| Branch Coverage | 33.66% (60947/181061) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789313760
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -365,4 +507,77 @@ private GetTabletStatsResponse getTabletStats(GetTabletStatsRequest request)
public List<OlapTable.Statistics> getCloudTableStats() {
return this.cloudTableStatsList;
}
+
+ public void addActiveTablets(List<Long> tabletIds) {
Review Comment:
will this method become bottleneck for high frequent load?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3870783993
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3892722577
# FE Regression Coverage Report
Increment line coverage ` 50.85% (119/234)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_a8d92e87c70482681b2768a6ff752b8f710024b4_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_a8d92e87c70482681b2768a6ff752b8f710024b4_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3890130780
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4074711764
<details>
<summary>TPC-H: <b>Total hot run time: 27244 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 72cf29d664b5bbc35c649177f43639ac08ee8732, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17660 4524 4336 4336
q2 q3 10726 790 528 528
q4 4713 371 260 260
q5 8119 1209 1013 1013
q6 216 174 146 146
q7 814 862 671 671
q8 10805 1469 1378 1378
q9 6811 4806 4733 4733
q10 6405 1923 1676 1676
q11 487 259 254 254
q12 748 585 476 476
q13 18049 2936 2182 2182
q14 236 228 217 217
q15 q16 737 764 674 674
q17 750 853 460 460
q18 6074 5399 5408 5399
q19 1131 993 629 629
q20 545 487 381 381
q21 4975 2117 1552 1552
q22 381 318 279 279
Total cold run time: 100382 ms
Total hot run time: 27244 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4684 4546 4585 4546
q2 q3 3889 4343 3849 3849
q4 865 1211 784 784
q5 4166 4553 4346 4346
q6 177 170 142 142
q7 1761 1679 1555 1555
q8 2520 2757 2609 2609
q9 7703 7494 7318 7318
q10 3832 3999 3595 3595
q11 545 452 457 452
q12 613 581 429 429
q13 2870 3172 2306 2306
q14 308 315 286 286
q15 q16 743 761 806 761
q17 1162 1361 1301 1301
q18 7282 6844 6799 6799
q19 910 916 954 916
q20 2176 2165 1998 1998
q21 3952 3492 3340 3340
q22 454 424 372 372
Total cold run time: 50612 ms
Total hot run time: 47704 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3859977980
# BE Regression && UT Coverage Report
Increment line coverage `58.82% (10/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.64% (25901/36155) |
| Line Coverage | 54.26% (270633/498758) |
| Region Coverage | 51.83% (225798/435653) |
| Branch Coverage | 53.25% (96815/181813) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3869868375
# BE UT Coverage Report
Increment line coverage `0.00% (0/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/ce9797d95d49229785e41d695ea9b1088772c73c_ce9797d95d49229785e41d695ea9b1088772c73c/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/ce9797d95d49229785e41d695ea9b1088772c73c_ce9797d95d49229785e41d695ea9b1088772c73c/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.73% (19445/36880) |
| Line Coverage | 36.22% (181025/499854) |
| Region Coverage | 32.58% (140466/431135) |
| Branch Coverage | 33.62% (60865/181045) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3852738217
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3858501254
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/a825e3aac89682ec52fb55780ca7284e6df70a8b_a825e3aac89682ec52fb55780ca7284e6df70a8b_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/a825e3aac89682ec52fb55780ca7284e6df70a8b_a825e3aac89682ec52fb55780ca7284e6df70a8b_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.29% (1792/2260) |
| Line Coverage | 64.75% (31830/49158) |
| Region Coverage | 65.39% (15877/24280) |
| Branch Coverage | 55.96% (8437/15078) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3858698318
# FE UT Coverage Report
Increment line coverage ` 14.41% (32/222)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789296517
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java:
##########
@@ -67,9 +69,15 @@ public class CloudReplica extends Replica implements GsonPostProcessable {
private long indexId = -1;
@SerializedName(value = "idx")
private long idx = -1;
+ @Getter
+ @Setter
+ long lastGetTabletStatsTime = 0;
+ @Getter
+ @Setter
+ int statsIntervalIndex = 0;
Review Comment:
what is this for?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789364637
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -122,16 +198,62 @@ private List<Long> getAllTabletStats() {
LOG.error("Error waiting for get tablet stats tasks to complete", e);
}
- LOG.info("finished to get tablet stat of all backends. cost: {} ms",
- (System.currentTimeMillis() - start));
+ LOG.info("finished to get tablet stats. getStatsTabletNum: {}, cost: {} ms",
+ getStatsTabletNum, (System.currentTimeMillis() - start));
return dbIds;
}
- private Future<Void> submitGetTabletStatsTask(GetTabletStatsRequest req) {
+ private void getActiveTabletStats(Set<Long> tablets) {
Review Comment:
consider adding a metricss: time taken and number of tablets, of this method
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3853741789
<details>
<summary>ClickBench: <b>Total hot run time: 28.85 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1, data reload: false
query1 0.05 0.04 0.04
query2 0.09 0.04 0.04
query3 0.25 0.09 0.09
query4 1.61 0.11 0.11
query5 0.27 0.25 0.26
query6 1.18 0.68 0.68
query7 0.03 0.02 0.02
query8 0.05 0.04 0.04
query9 0.56 0.50 0.49
query10 0.55 0.56 0.55
query11 0.14 0.10 0.09
query12 0.13 0.10 0.11
query13 0.63 0.62 0.63
query14 1.06 1.06 1.07
query15 0.87 0.86 0.88
query16 0.39 0.39 0.39
query17 1.05 1.12 1.12
query18 0.22 0.21 0.21
query19 2.07 2.00 2.04
query20 0.02 0.02 0.02
query21 15.42 0.28 0.14
query22 4.96 0.06 0.05
query23 15.93 0.29 0.10
query24 1.35 0.69 0.24
query25 0.10 0.08 0.07
query26 0.15 0.14 0.13
query27 0.05 0.05 0.06
query28 3.98 1.15 0.96
query29 12.55 3.92 3.17
query30 0.28 0.13 0.11
query31 2.84 0.64 0.40
query32 3.25 0.60 0.50
query33 3.29 3.22 3.25
query34 15.98 5.38 4.70
query35 4.79 5.31 5.39
query36 0.68 0.59 0.53
query37 0.11 0.07 0.06
query38 0.07 0.04 0.04
query39 0.04 0.03 0.03
query40 0.20 0.18 0.17
query41 0.08 0.03 0.03
query42 0.04 0.03 0.03
query43 0.04 0.03 0.03
Total cold run time: 97.4 s
Total hot run time: 28.85 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3858419272
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3860024168
# FE Regression Coverage Report
Increment line coverage ` 55.41% (123/222)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_a825e3aac89682ec52fb55780ca7284e6df70a8b_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3890348868
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/a8d92e87c70482681b2768a6ff752b8f710024b4_a8d92e87c70482681b2768a6ff752b8f710024b4_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/a8d92e87c70482681b2768a6ff752b8f710024b4_a8d92e87c70482681b2768a6ff752b8f710024b4_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.32% (1795/2263) |
| Line Coverage | 64.81% (31957/49311) |
| Region Coverage | 65.48% (15938/24339) |
| Branch Coverage | 55.99% (8473/15132) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3871013732
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/631284d5b22f079e4928cbab8f35eea4c670ef78_631284d5b22f079e4928cbab8f35eea4c670ef78_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/631284d5b22f079e4928cbab8f35eea4c670ef78_631284d5b22f079e4928cbab8f35eea4c670ef78_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.35% (1795/2262) |
| Line Coverage | 64.86% (31958/49269) |
| Region Coverage | 65.56% (15945/24323) |
| Branch Coverage | 56.05% (8472/15114) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3854296795
# BE UT Coverage Report
Increment line coverage `0.00% (0/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.55% (19372/36861) |
| Line Coverage | 36.04% (179944/499291) |
| Region Coverage | 32.41% (139516/430471) |
| Branch Coverage | 33.40% (60409/180850) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3859618077
# BE UT Coverage Report
Increment line coverage `0.00% (0/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/a825e3aac89682ec52fb55780ca7284e6df70a8b_a825e3aac89682ec52fb55780ca7284e6df70a8b/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/a825e3aac89682ec52fb55780ca7284e6df70a8b_a825e3aac89682ec52fb55780ca7284e6df70a8b/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.61% (19409/36894) |
| Line Coverage | 36.11% (180563/499982) |
| Region Coverage | 32.44% (139918/431269) |
| Branch Coverage | 33.47% (60621/181105) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3870265985
# FE Regression Coverage Report
Increment line coverage ` 54.71% (122/223)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_ce9797d95d49229785e41d695ea9b1088772c73c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_ce9797d95d49229785e41d695ea9b1088772c73c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3871376322
<details>
<summary>TPC-H: <b>Total hot run time: 30646 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 631284d5b22f079e4928cbab8f35eea4c670ef78, data reload: false
------ Round 1 ----------------------------------
q1 17621 4445 4247 4247
q2 2033 365 256 256
q3 10135 1316 726 726
q4 10201 787 304 304
q5 7500 2231 1964 1964
q6 237 183 147 147
q7 868 761 610 610
q8 9279 1400 1216 1216
q9 5017 4630 4730 4630
q10 6877 1931 1570 1570
q11 505 308 296 296
q12 397 382 226 226
q13 17816 4145 3226 3226
q14 229 233 216 216
q15 873 814 795 795
q16 694 737 634 634
q17 910 895 427 427
q18 7041 5900 6588 5900
q19 1534 1055 632 632
q20 550 569 406 406
q21 2821 1934 1976 1934
q22 382 338 284 284
Total cold run time: 103520 ms
Total hot run time: 30646 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4622 4641 4510 4510
q2 317 405 266 266
q3 2251 2907 2545 2545
q4 1413 1839 1490 1490
q5 4785 4692 4517 4517
q6 223 176 137 137
q7 2028 1930 1782 1782
q8 2567 2373 2374 2373
q9 7893 7414 7440 7414
q10 2847 3054 2663 2663
q11 569 481 464 464
q12 701 835 629 629
q13 3996 4410 3565 3565
q14 294 287 256 256
q15 844 775 771 771
q16 636 683 639 639
q17 1092 1229 1235 1229
q18 7704 7480 7320 7320
q19 852 799 835 799
q20 1987 1994 1931 1931
q21 4541 4234 4154 4154
q22 552 536 491 491
Total cold run time: 52714 ms
Total hot run time: 49945 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3872296363
# BE Regression && UT Coverage Report
Increment line coverage `58.82% (10/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.77% (25937/36138) |
| Line Coverage | 54.40% (271256/498675) |
| Region Coverage | 51.80% (225635/435562) |
| Branch Coverage | 53.23% (96757/181765) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3871450364
<details>
<summary>ClickBench: <b>Total hot run time: 28.38 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 631284d5b22f079e4928cbab8f35eea4c670ef78, data reload: false
query1 0.05 0.05 0.05
query2 0.09 0.05 0.05
query3 0.26 0.09 0.08
query4 1.60 0.11 0.12
query5 0.27 0.24 0.25
query6 1.16 0.70 0.67
query7 0.03 0.03 0.03
query8 0.06 0.04 0.04
query9 0.57 0.50 0.49
query10 0.53 0.54 0.54
query11 0.14 0.09 0.09
query12 0.14 0.11 0.10
query13 0.65 0.62 0.61
query14 1.07 1.06 1.07
query15 0.87 0.87 0.87
query16 0.39 0.41 0.38
query17 1.14 1.12 1.11
query18 0.22 0.20 0.21
query19 2.04 2.00 2.05
query20 0.02 0.02 0.01
query21 15.40 0.26 0.15
query22 4.85 0.06 0.06
query23 15.88 0.28 0.11
query24 2.47 0.34 0.30
query25 0.09 0.13 0.05
query26 0.14 0.13 0.14
query27 0.06 0.05 0.06
query28 3.37 1.16 0.97
query29 12.57 3.90 3.13
query30 0.29 0.13 0.12
query31 2.81 0.65 0.42
query32 3.24 0.60 0.49
query33 3.23 3.24 3.27
query34 16.41 5.41 4.75
query35 4.82 4.79 4.84
query36 0.65 0.50 0.50
query37 0.12 0.07 0.07
query38 0.08 0.05 0.05
query39 0.04 0.03 0.03
query40 0.20 0.15 0.15
query41 0.08 0.03 0.03
query42 0.05 0.03 0.03
query43 0.04 0.03 0.04
Total cold run time: 98.19 s
Total hot run time: 28.38 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789277484
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -3580,6 +3580,10 @@ public static int metaServiceRpcRetryTimes() {
+ "model is set to 300 times, which is approximately 5 minutes by default."})
public static int auto_start_wait_to_resume_times = 300;
+ @ConfField(description = {"存算分离模式下 Sync tablet stats task 的最大并发数。",
Review Comment:
what is the difference to the existing config `max_get_tablet_stat_task_threads_num`
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3853182646
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.29% (1792/2260) |
| Line Coverage | 64.75% (31829/49158) |
| Region Coverage | 65.43% (15886/24280) |
| Branch Coverage | 55.92% (8432/15078) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2798144307
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -365,4 +507,77 @@ private GetTabletStatsResponse getTabletStats(GetTabletStatsRequest request)
public List<OlapTable.Statistics> getCloudTableStats() {
return this.cloudTableStatsList;
}
+
+ public void addActiveTablets(List<Long> tabletIds) {
+ if (tabletIds == null || tabletIds.isEmpty()) {
+ return;
+ }
+ synchronized (activeTablets) {
+ activeTablets.addAll(tabletIds);
+ }
+ }
+
+ // master FE send update tablet stats rpc to other FEs
+ private void pushTabletStats(GetTabletStatsResponse response) {
+ List<Frontend> frontends = getFrontends();
+ if (frontends == null || frontends.isEmpty()) {
Review Comment:
yes, only master calls it
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3872373643
# FE Regression Coverage Report
Increment line coverage ` 55.65% (128/230)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3869146205
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789496725
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -60,11 +101,42 @@ public CloudTabletStatMgr() {
@Override
protected void runAfterCatalogReady() {
LOG.info("cloud tablet stat begin");
- List<Long> dbIds = getAllTabletStats();
+ // get stats for active tablets
+ Set<Long> copiedTablets;
+ synchronized (activeTablets) {
+ copiedTablets = new HashSet<>(activeTablets);
+ activeTablets.clear();
+ }
+ getActiveTabletStats(copiedTablets);
+
+ // get stats by interval
+ List<Long> dbIds = getAllTabletStats(cloudTablet -> {
+ if (copiedTablets.contains(cloudTablet.getId())) {
+ return false;
+ }
+ List<Replica> replicas = Env.getCurrentInvertedIndex().getReplicas(cloudTablet.getId());
+ if (replicas == null || replicas.isEmpty()) {
+ return false;
+ }
+ CloudReplica cloudReplica = (CloudReplica) replicas.get(0);
+ int index = cloudReplica.getStatsIntervalIndex();
+ if (index >= DEFAULT_INTERVAL_LADDER_MS.length) {
+ LOG.warn("get tablet stats interval index out of range, tabletId: {}, index: {}",
+ cloudTablet.getId(), index);
+ index = DEFAULT_INTERVAL_LADDER_MS.length - 1;
+ }
+ long interval = DEFAULT_INTERVAL_LADDER_MS[index];
+ if (interval == Long.MAX_VALUE
+ || System.currentTimeMillis() - cloudReplica.getLastGetTabletStatsTime() < interval) {
+ return false;
+ }
+ return true;
+ });
updateStatInfo(dbIds);
}
- private List<Long> getAllTabletStats() {
+ private List<Long> getAllTabletStats(Function<CloudTablet, Boolean> filter) {
Review Comment:
should we add some kind mechanism like forcing get all tablet stats for emergency or fallback to the original logic in case of bugs.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3869646407
<details>
<summary>ClickBench: <b>Total hot run time: 28.07 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit ce9797d95d49229785e41d695ea9b1088772c73c, data reload: false
query1 0.05 0.04 0.04
query2 0.13 0.07 0.06
query3 0.33 0.08 0.08
query4 1.60 0.11 0.10
query5 0.29 0.25 0.25
query6 1.14 0.66 0.63
query7 0.03 0.02 0.03
query8 0.07 0.05 0.06
query9 0.59 0.50 0.50
query10 0.54 0.54 0.55
query11 0.26 0.13 0.14
query12 0.26 0.14 0.15
query13 0.62 0.62 0.60
query14 0.98 0.96 0.97
query15 0.92 0.83 0.82
query16 0.40 0.38 0.41
query17 1.02 1.01 1.00
query18 0.28 0.22 0.23
query19 1.92 1.88 1.87
query20 0.03 0.01 0.01
query21 15.41 0.34 0.30
query22 4.92 0.12 0.13
query23 15.33 0.47 0.28
query24 2.15 0.57 0.40
query25 0.12 0.11 0.12
query26 0.20 0.18 0.18
query27 0.13 0.11 0.10
query28 3.54 1.15 0.98
query29 12.50 4.12 3.25
query30 0.32 0.13 0.10
query31 2.80 0.69 0.43
query32 3.24 0.62 0.50
query33 3.05 3.11 3.01
query34 16.48 5.13 4.47
query35 4.48 4.43 4.51
query36 0.62 0.51 0.49
query37 0.31 0.09 0.08
query38 0.29 0.06 0.06
query39 0.07 0.05 0.04
query40 0.20 0.18 0.17
query41 0.14 0.07 0.06
query42 0.08 0.05 0.05
query43 0.07 0.06 0.06
Total cold run time: 97.91 s
Total hot run time: 28.07 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3852987368
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2798152019
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -3580,6 +3580,10 @@ public static int metaServiceRpcRetryTimes() {
+ "model is set to 300 times, which is approximately 5 minutes by default."})
public static int auto_start_wait_to_resume_times = 300;
+ @ConfField(description = {"存算分离模式下 Sync tablet stats task 的最大并发数。",
Review Comment:
`max_get_tablet_stat_task_threads_num`: fe get tablet stats from ms.
`cloud_sync_tablet_stats_task_threads_num`: master fe push active tablet stats to other fes.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3869251967
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/ce9797d95d49229785e41d695ea9b1088772c73c_ce9797d95d49229785e41d695ea9b1088772c73c_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/ce9797d95d49229785e41d695ea9b1088772c73c_ce9797d95d49229785e41d695ea9b1088772c73c_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.35% (1795/2262) |
| Line Coverage | 64.87% (31960/49269) |
| Region Coverage | 65.53% (15939/24323) |
| Branch Coverage | 56.08% (8476/15114) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3870186651
# BE Regression && UT Coverage Report
Increment line coverage `58.82% (10/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_ce9797d95d49229785e41d695ea9b1088772c73c_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_ce9797d95d49229785e41d695ea9b1088772c73c_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.46% (26542/36132) |
| Line Coverage | 56.55% (281866/498479) |
| Region Coverage | 54.10% (235591/435440) |
| Branch Coverage | 55.84% (101455/181677) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789397916
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java:
##########
@@ -67,9 +69,15 @@ public class CloudReplica extends Replica implements GsonPostProcessable {
private long indexId = -1;
@SerializedName(value = "idx")
private long idx = -1;
+ @Getter
+ @Setter
+ long lastGetTabletStatsTime = 0;
+ @Getter
+ @Setter
+ int statsIntervalIndex = 0;
Review Comment:
better add some comment
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3853631119
<details>
<summary>TPC-H: <b>Total hot run time: 31144 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1, data reload: false
------ Round 1 ----------------------------------
q1 17617 4462 4314 4314
q2 2012 330 226 226
q3 10176 1273 740 740
q4 10224 904 319 319
q5 8207 2160 1960 1960
q6 225 176 148 148
q7 868 738 576 576
q8 9288 1390 1046 1046
q9 5356 4841 4973 4841
q10 6874 1972 1558 1558
q11 499 298 277 277
q12 372 373 220 220
q13 17764 4036 3213 3213
q14 238 238 215 215
q15 890 822 787 787
q16 698 700 617 617
q17 644 835 455 455
q18 7004 6471 7452 6471
q19 1146 1017 636 636
q20 413 375 257 257
q21 2919 2331 1983 1983
q22 356 326 285 285
Total cold run time: 103790 ms
Total hot run time: 31144 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4597 4755 4506 4506
q2 256 336 274 274
q3 2298 2893 2469 2469
q4 1492 1869 1488 1488
q5 4837 4459 4595 4459
q6 225 175 142 142
q7 1989 1924 1771 1771
q8 2683 2535 2371 2371
q9 7334 7656 7618 7618
q10 2730 3014 2565 2565
q11 531 476 442 442
q12 674 737 643 643
q13 3913 4261 3284 3284
q14 268 281 262 262
q15 830 832 769 769
q16 635 697 635 635
q17 1096 1252 1307 1252
q18 7343 7322 7283 7283
q19 833 812 813 812
q20 1925 2011 1873 1873
q21 4513 4187 4091 4091
q22 568 555 486 486
Total cold run time: 51570 ms
Total hot run time: 49495 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3854583367
# BE Regression && UT Coverage Report
Increment line coverage `58.82% (10/17)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.69% (25899/36124) |
| Line Coverage | 54.31% (270501/498103) |
| Region Coverage | 51.65% (224617/434889) |
| Branch Coverage | 53.19% (96585/181582) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4073835655
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2945810367
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -338,24 +463,53 @@ private void updateStatInfo(List<Long> dbIds) {
(System.currentTimeMillis() - start));
}
- private void updateTabletStat(GetTabletStatsResponse response) {
+ private void updateTabletStat(GetTabletStatsResponse response, boolean activeUpdate) {
TabletInvertedIndex invertedIndex = Env.getCurrentInvertedIndex();
for (TabletStatsPB stat : response.getTabletStatsList()) {
List<Replica> replicas = invertedIndex.getReplicasByTabletId(stat.getIdx().getTabletId());
if (replicas == null || replicas.isEmpty() || replicas.get(0) == null) {
continue;
}
Replica replica = replicas.get(0);
+ boolean statsChanged = replica.getDataSize() != stat.getDataSize()
+ || replica.getRowsetCount() != stat.getNumRowsets()
+ || replica.getSegmentCount() != stat.getNumSegments()
+ || replica.getRowCount() != stat.getNumRows()
+ || replica.getLocalInvertedIndexSize() != stat.getIndexSize()
+ || replica.getLocalSegmentSize() != stat.getSegmentSize();
replica.setDataSize(stat.getDataSize());
replica.setRowsetCount(stat.getNumRowsets());
replica.setSegmentCount(stat.getNumSegments());
replica.setRowCount(stat.getNumRows());
replica.setLocalInvertedIndexSize(stat.getIndexSize());
replica.setLocalSegmentSize(stat.getSegmentSize());
+
+ CloudReplica cloudReplica = (CloudReplica) replica;
+ cloudReplica.setLastGetTabletStatsTime(System.currentTimeMillis());
+ int statsIntervalIndex = cloudReplica.getStatsIntervalIndex();
Review Comment:
**[Low - Concurrency]** The read-modify-write pattern on `statsIntervalIndex` is not atomic:
```java
int statsIntervalIndex = cloudReplica.getStatsIntervalIndex(); // READ
// ... compute new value ...
cloudReplica.setStatsIntervalIndex(statsIntervalIndex); // WRITE
```
On a follower, if the daemon's `getAllTabletStats` interval path (which calls `updateTabletStat(resp, false)`) and the `syncTabletStats` push path (which calls `updateTabletStat(resp, true)`) process the same tablet concurrently, the daemon's increment could overwrite the push's reset-to-0.
The practical impact is low (at worst, a tablet stays at a higher interval for one extra cycle), but making the fields `volatile` and documenting this accepted race would be cleaner.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -1636,10 +1647,14 @@ private void commitTransactionWithSubTxns(long dbId, List<Table> tableList, long
.map(c -> new TabletCommitInfo(c.getTabletId(), c.getBackendId()))
.collect(Collectors.toList())))
.build());
+ for (TTabletCommitInfo tabletCommitInfo : subTransactionState.getTabletCommitInfos()) {
+ tabletIds.add(tabletCommitInfo.getTabletId());
+ }
}
final CommitTxnRequest commitTxnRequest = builder.build();
- executeCommitTxnRequest(commitTxnRequest, transactionId, false, null);
+ executeCommitTxnRequest(commitTxnRequest, transactionId, false, null,
+ tabletIds.stream().collect(Collectors.toList()));
}
Review Comment:
**[Nit - Performance]** `tabletIds.stream().collect(Collectors.toList())` can be simplified to `new ArrayList<>(tabletIds)` since `tabletIds` is already a `Set<Long>`. The stream is unnecessary overhead.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java:
##########
@@ -67,9 +69,23 @@ public class CloudReplica extends Replica implements GsonPostProcessable {
private long indexId = -1;
@SerializedName(value = "idx")
private long idx = -1;
+ // last time to get tablet stats
+ @Getter
+ @Setter
+ long lastGetTabletStatsTime = 0;
+ /**
Review Comment:
**[Medium - Concurrency]** Both `lastGetTabletStatsTime` and `statsIntervalIndex` are read by the daemon thread in `CloudTabletStatMgr.runAfterCatalogReady()` and written by `GET_TABLET_STATS_THREAD_POOL` / `SYNC_TABLET_STATS_THREAD_POOL` threads in `updateTabletStat()`. Since these are plain fields (not `volatile`), the JMM does not guarantee visibility across threads.
Concrete scenario: On a follower FE, the master pushes tablet stats via `syncTabletStats` -> `updateTabletStat(response, true)`, which sets `statsIntervalIndex = 0` and `lastGetTabletStatsTime = now`. The daemon thread running `runAfterCatalogReady()` may read stale values (old `statsIntervalIndex` and old `lastGetTabletStatsTime`), causing it to fetch the same tablet stats again from meta_service — exactly the redundancy this PR aims to eliminate.
Recommend: Mark both fields as `volatile`:
```java
volatile long lastGetTabletStatsTime = 0;
volatile int statsIntervalIndex = 0;
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4073992528
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789352634
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -60,11 +101,42 @@ public CloudTabletStatMgr() {
@Override
protected void runAfterCatalogReady() {
LOG.info("cloud tablet stat begin");
- List<Long> dbIds = getAllTabletStats();
+ // get stats for active tablets
+ Set<Long> copiedTablets;
+ synchronized (activeTablets) {
+ copiedTablets = new HashSet<>(activeTablets);
Review Comment:
better using algorithm like `swap` to get rid of copy, e.g. just new an instance of `activeTablets`, because `addActiveTablet()` may be blocked for long if there are millions of tablets.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789368426
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -122,16 +198,62 @@ private List<Long> getAllTabletStats() {
LOG.error("Error waiting for get tablet stats tasks to complete", e);
}
- LOG.info("finished to get tablet stat of all backends. cost: {} ms",
- (System.currentTimeMillis() - start));
+ LOG.info("finished to get tablet stats. getStatsTabletNum: {}, cost: {} ms",
+ getStatsTabletNum, (System.currentTimeMillis() - start));
return dbIds;
}
- private Future<Void> submitGetTabletStatsTask(GetTabletStatsRequest req) {
+ private void getActiveTabletStats(Set<Long> tablets) {
+ List<Long> tabletIds = new ArrayList<>(tablets);
+ Collections.sort(tabletIds);
+ List<TabletMeta> tabletMetas = Env.getCurrentInvertedIndex().getTabletMetaList(tabletIds);
+ long start = System.currentTimeMillis();
+ List<Future<Void>> futures = new ArrayList<>();
+ GetTabletStatsRequest.Builder builder =
+ GetTabletStatsRequest.newBuilder().setRequestIp(FrontendOptions.getLocalHostAddressCached());
+ long activeTabletNum = 0;
Review Comment:
should we also log before RPC request?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2798139570
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -122,16 +198,62 @@ private List<Long> getAllTabletStats() {
LOG.error("Error waiting for get tablet stats tasks to complete", e);
}
- LOG.info("finished to get tablet stat of all backends. cost: {} ms",
- (System.currentTimeMillis() - start));
+ LOG.info("finished to get tablet stats. getStatsTabletNum: {}, cost: {} ms",
+ getStatsTabletNum, (System.currentTimeMillis() - start));
return dbIds;
}
- private Future<Void> submitGetTabletStatsTask(GetTabletStatsRequest req) {
+ private void getActiveTabletStats(Set<Long> tablets) {
Review Comment:
there is a log
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3859036557
<details>
<summary>ClickBench: <b>Total hot run time: 28.37 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit a825e3aac89682ec52fb55780ca7284e6df70a8b, data reload: false
query1 0.05 0.05 0.05
query2 0.09 0.04 0.04
query3 0.26 0.09 0.08
query4 1.60 0.12 0.11
query5 0.27 0.26 0.24
query6 1.16 0.67 0.68
query7 0.03 0.03 0.03
query8 0.06 0.04 0.05
query9 0.58 0.51 0.50
query10 0.55 0.54 0.55
query11 0.14 0.09 0.10
query12 0.14 0.11 0.10
query13 0.63 0.61 0.62
query14 1.06 1.07 1.06
query15 0.88 0.86 0.89
query16 0.42 0.38 0.38
query17 1.09 1.14 1.15
query18 0.23 0.24 0.20
query19 2.06 2.02 2.00
query20 0.02 0.01 0.02
query21 15.41 0.28 0.15
query22 5.41 0.05 0.06
query23 16.14 0.29 0.11
query24 1.92 0.66 0.27
query25 0.09 0.07 0.06
query26 0.15 0.14 0.14
query27 0.09 0.06 0.06
query28 4.65 1.15 0.97
query29 12.59 3.83 3.20
query30 0.28 0.12 0.12
query31 2.81 0.63 0.40
query32 3.23 0.60 0.50
query33 3.19 3.28 3.25
query34 16.33 5.38 4.69
query35 4.87 4.77 4.88
query36 0.64 0.51 0.49
query37 0.12 0.07 0.07
query38 0.07 0.04 0.04
query39 0.05 0.03 0.03
query40 0.19 0.18 0.15
query41 0.08 0.03 0.03
query42 0.04 0.03 0.03
query43 0.04 0.04 0.04
Total cold run time: 99.71 s
Total hot run time: 28.37 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4021381111
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4022022052
<details>
<summary>TPC-DS: <b>Total hot run time: 153917 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 0b3e7be17dc0aec4ac9c0b68b418b6bbdaec5a26, data reload: false
query5 4326 646 550 550
query6 340 228 198 198
query7 4224 467 266 266
query8 337 250 235 235
query9 8728 2809 2716 2716
query10 533 371 349 349
query11 7309 5876 5654 5654
query12 193 128 128 128
query13 1254 462 348 348
query14 5574 3880 3605 3605
query14_1 2872 2843 2835 2835
query15 212 194 175 175
query16 1003 536 451 451
query17 925 715 644 644
query18 2445 459 365 365
query19 218 218 187 187
query20 138 134 134 134
query21 230 149 134 134
query22 4837 4983 4893 4893
query23 16818 16114 15781 15781
query23_1 15988 15864 15835 15835
query24 7742 1684 1293 1293
query24_1 1291 1259 1296 1259
query25 627 539 498 498
query26 1248 272 160 160
query27 2776 487 301 301
query28 4496 1885 1923 1885
query29 809 557 474 474
query30 317 248 206 206
query31 1348 1282 1215 1215
query32 81 74 73 73
query33 507 337 283 283
query34 915 912 556 556
query35 631 694 597 597
query36 1091 1135 983 983
query37 129 93 81 81
query38 2929 2950 2900 2900
query39 1031 877 852 852
query39_1 841 847 812 812
query40 230 149 138 138
query41 63 62 59 59
query42 303 305 303 303
query43 239 245 220 220
query44
query45 207 187 179 179
query46 880 996 611 611
query47 2107 2144 2050 2050
query48 315 310 231 231
query49 626 460 431 431
query50 699 281 219 219
query51 4143 4131 4104 4104
query52 286 296 289 289
query53 292 337 279 279
query54 298 279 270 270
query55 96 88 87 87
query56 320 314 313 313
query57 1353 1334 1321 1321
query58 295 281 288 281
query59 1428 1444 1255 1255
query60 339 334 325 325
query61 153 155 147 147
query62 615 591 546 546
query63 308 280 283 280
query64 4982 1280 970 970
query65
query66 1478 458 360 360
query67 16489 16512 16518 16512
query68
query69 389 314 278 278
query70 979 979 945 945
query71 349 311 304 304
query72 2785 2653 2437 2437
query73 559 559 334 334
query74 10110 9944 9767 9767
query75 2897 2749 2456 2456
query76 2281 1050 684 684
query77 369 384 294 294
query78 11271 11410 10640 10640
query79 1896 814 604 604
query80 1470 671 550 550
query81 583 278 258 258
query82 1009 158 120 120
query83 334 276 250 250
query84 291 122 98 98
query85 897 484 452 452
query86 417 313 328 313
query87 3170 3141 3014 3014
query88 3563 2709 2669 2669
query89 436 378 357 357
query90 2045 180 174 174
query91 162 162 139 139
query92 79 74 73 73
query93 1017 836 513 513
query94 640 317 302 302
query95 601 357 323 323
query96 646 521 225 225
query97 2487 2542 2409 2409
query98 243 224 224 224
query99 995 1006 912 912
Total cold run time: 235382 ms
Total hot run time: 153917 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3890564893
# FE UT Coverage Report
Increment line coverage ` 13.25% (31/234)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_a8d92e87c70482681b2768a6ff752b8f710024b4/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_a8d92e87c70482681b2768a6ff752b8f710024b4/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2798141415
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/CloudTabletStatMgr.java:
##########
@@ -122,16 +198,62 @@ private List<Long> getAllTabletStats() {
LOG.error("Error waiting for get tablet stats tasks to complete", e);
}
- LOG.info("finished to get tablet stat of all backends. cost: {} ms",
- (System.currentTimeMillis() - start));
+ LOG.info("finished to get tablet stats. getStatsTabletNum: {}, cost: {} ms",
+ getStatsTabletNum, (System.currentTimeMillis() - start));
return dbIds;
}
- private Future<Void> submitGetTabletStatsTask(GetTabletStatsRequest req) {
+ private void getActiveTabletStats(Set<Long> tablets) {
+ List<Long> tabletIds = new ArrayList<>(tablets);
+ Collections.sort(tabletIds);
+ List<TabletMeta> tabletMetas = Env.getCurrentInvertedIndex().getTabletMetaList(tabletIds);
+ long start = System.currentTimeMillis();
+ List<Future<Void>> futures = new ArrayList<>();
+ GetTabletStatsRequest.Builder builder =
+ GetTabletStatsRequest.newBuilder().setRequestIp(FrontendOptions.getLocalHostAddressCached());
+ long activeTabletNum = 0;
Review Comment:
there is metric for rpc
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3871192935
# FE UT Coverage Report
Increment line coverage ` 14.35% (33/230)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_631284d5b22f079e4928cbab8f35eea4c670ef78/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4074152970
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/72cf29d664b5bbc35c649177f43639ac08ee8732_72cf29d664b5bbc35c649177f43639ac08ee8732_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/72cf29d664b5bbc35c649177f43639ac08ee8732_72cf29d664b5bbc35c649177f43639ac08ee8732_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.15% (1796/2269) |
| Line Coverage | 64.47% (32281/50072) |
| Region Coverage | 65.38% (16167/24727) |
| Branch Coverage | 55.81% (8613/15434) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3854673182
# FE Regression Coverage Report
Increment line coverage ` 52.49% (116/221)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60543_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60543_c1abd4d6049f7ed68ce7ba7b8a48d81d96e9ddd1_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3858942993
<details>
<summary>TPC-H: <b>Total hot run time: 30504 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit a825e3aac89682ec52fb55780ca7284e6df70a8b, data reload: false
------ Round 1 ----------------------------------
q1 17603 4459 4316 4316
q2 2043 386 235 235
q3 10125 1322 745 745
q4 10227 786 312 312
q5 8317 2246 1948 1948
q6 221 176 147 147
q7 891 743 619 619
q8 9285 1385 1177 1177
q9 4882 4683 4575 4575
q10 6910 1946 1562 1562
q11 520 311 298 298
q12 403 381 234 234
q13 17826 4088 3298 3298
q14 244 240 215 215
q15 911 809 811 809
q16 687 668 621 621
q17 710 799 560 560
q18 6740 5719 5852 5719
q19 1177 1024 632 632
q20 500 492 383 383
q21 2572 1833 1823 1823
q22 366 326 276 276
Total cold run time: 103160 ms
Total hot run time: 30504 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4399 4343 4352 4343
q2 261 338 258 258
q3 2104 2683 2214 2214
q4 1361 1736 1287 1287
q5 4284 4224 4209 4209
q6 221 181 135 135
q7 1844 1839 1681 1681
q8 2735 2656 2461 2461
q9 7676 7562 7578 7562
q10 2976 3130 2697 2697
q11 536 481 450 450
q12 694 736 635 635
q13 4016 4519 3430 3430
q14 298 317 292 292
q15 839 812 786 786
q16 674 724 670 670
q17 1141 1323 1416 1323
q18 8184 7982 8015 7982
q19 886 907 872 872
q20 2058 2250 2077 2077
q21 4810 4431 4451 4431
q22 555 555 514 514
Total cold run time: 52552 ms
Total hot run time: 50309 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-3869584132
<details>
<summary>TPC-H: <b>Total hot run time: 30338 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ce9797d95d49229785e41d695ea9b1088772c73c, data reload: false
------ Round 1 ----------------------------------
q1 17637 4470 4320 4320
q2 2022 352 250 250
q3 10416 1328 733 733
q4 10322 783 306 306
q5 9486 2211 1915 1915
q6 237 177 146 146
q7 903 738 613 613
q8 9298 1367 1115 1115
q9 4952 4689 4592 4592
q10 6876 1949 1545 1545
q11 525 305 291 291
q12 392 378 227 227
q13 17817 4030 3201 3201
q14 230 232 223 223
q15 918 809 812 809
q16 688 676 634 634
q17 881 784 576 576
q18 6510 5814 5737 5737
q19 1386 1007 646 646
q20 500 487 388 388
q21 2562 1833 1797 1797
q22 362 320 274 274
Total cold run time: 104920 ms
Total hot run time: 30338 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4476 4386 4369 4369
q2 261 349 258 258
q3 2104 2666 2213 2213
q4 1348 1732 1298 1298
q5 4339 4262 4308 4262
q6 213 178 140 140
q7 1834 1798 1880 1798
q8 2667 2408 2450 2408
q9 7530 7461 7551 7461
q10 2850 3026 2681 2681
q11 547 488 471 471
q12 687 761 602 602
q13 3877 4427 3712 3712
q14 310 305 289 289
q15 857 804 835 804
q16 692 732 692 692
q17 1175 1365 1352 1352
q18 8252 8154 8198 8154
q19 909 868 890 868
q20 2108 2164 2292 2164
q21 4819 4385 4116 4116
q22 581 532 500 500
Total cold run time: 52436 ms
Total hot run time: 50612 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #60543:
URL: https://github.com/apache/doris/pull/60543#discussion_r2789404300
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4707,6 +4722,33 @@ public TGetOlapTableMetaResult getOlapTableMeta(TGetOlapTableMetaRequest request
}
}
+ @Override
+ public TStatus syncCloudTabletStats(TSyncCloudTabletStatsRequest request)
Review Comment:
add some comments
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improve](cloud) cloud reduce get_tablet_stats rpc to meta_service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60543:
URL: https://github.com/apache/doris/pull/60543#issuecomment-4021961279
<details>
<summary>TPC-H: <b>Total hot run time: 28090 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 0b3e7be17dc0aec4ac9c0b68b418b6bbdaec5a26, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17699 4500 4389 4389
q2 q3 10735 814 524 524
q4 4724 362 259 259
q5 8189 1221 1006 1006
q6 240 175 147 147
q7 805 859 678 678
q8 10657 1512 1374 1374
q9 6778 4791 4769 4769
q10 6499 1925 1758 1758
q11 464 275 254 254
q12 752 579 479 479
q13 18111 2963 2189 2189
q14 234 230 222 222
q15 955 806 834 806
q16 740 734 684 684
q17 721 872 417 417
q18 6045 5410 5320 5320
q19 1126 1016 613 613
q20 510 520 401 401
q21 5187 1948 1533 1533
q22 379 325 268 268
Total cold run time: 101550 ms
Total hot run time: 28090 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 4722 4572 4512 4512
q2 q3 3900 4370 3826 3826
q4 915 1203 775 775
q5 4085 4395 4364 4364
q6 191 178 142 142
q7 1790 1673 1508 1508
q8 2542 2752 2831 2752
q9 7652 7527 7541 7527
q10 3838 3974 3617 3617
q11 502 428 420 420
q12 495 592 442 442
q13 2872 3199 2402 2402
q14 283 298 274 274
q15 887 827 841 827
q16 743 777 716 716
q17 1165 1502 1335 1335
q18 7444 6962 6863 6863
q19 957 939 1096 939
q20 2115 2178 2006 2006
q21 4057 3552 3347 3347
q22 454 441 364 364
Total cold run time: 51609 ms
Total hot run time: 48958 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org