You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "SpriCoder (via GitHub)" <gi...@apache.org> on 2023/02/03 09:35:48 UTC
[GitHub] [iotdb] SpriCoder opened a new pull request, #8974: [IOTDB-5458] Add Performance Overview Metrics and Dashboard
SpriCoder opened a new pull request, #8974:
URL: https://github.com/apache/iotdb/pull/8974
Update later
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #8974: [IOTDB-5458] Add Performance Overview Metrics and Dashboard
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #8974:
URL: https://github.com/apache/iotdb/pull/8974#discussion_r1096613186
##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/DataRegion.java:
##########
@@ -1154,7 +1155,10 @@ private void insertToTsFileProcessor(
long globalLatestFlushTime =
lastFlushTimeMap.getGlobalFlushedTime(insertRowNode.getDevicePath().getFullPath());
+ long startTime = System.nanoTime();
tryToUpdateInsertLastCache(insertRowNode, globalLatestFlushTime);
+ PerformanceOverviewMetricsManager.getInstance()
Review Comment:
What's this? Can this metric item be merged with writing to memtable? Many users understand writing to wal and memtable, but not to lastCache
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SpriCoder closed pull request #8974: [IOTDB-5458] Add Performance Overview Metrics and Dashboard
Posted by "SpriCoder (via GitHub)" <gi...@apache.org>.
SpriCoder closed pull request #8974: [IOTDB-5458] Add Performance Overview Metrics and Dashboard
URL: https://github.com/apache/iotdb/pull/8974
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #8974: [IOTDB-5458] Add Performance Overview Metrics and Dashboard
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #8974:
URL: https://github.com/apache/iotdb/pull/8974#discussion_r1096613186
##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/DataRegion.java:
##########
@@ -1154,7 +1155,10 @@ private void insertToTsFileProcessor(
long globalLatestFlushTime =
lastFlushTimeMap.getGlobalFlushedTime(insertRowNode.getDevicePath().getFullPath());
+ long startTime = System.nanoTime();
tryToUpdateInsertLastCache(insertRowNode, globalLatestFlushTime);
+ PerformanceOverviewMetricsManager.getInstance()
Review Comment:
What's this?
##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/DataRegion.java:
##########
@@ -1727,8 +1731,11 @@ public void readUnlock() {
/** lock the write lock of the insert lock */
public void writeLock(String holder) {
+ final long startTime = System.nanoTime();
Review Comment:
I suspect that the metric added here does not reflect the formal write request locking time, since the method is likely to be called by a lot of code, perhaps we can just add monitoring to a few calls in the red box.
<img width="1093" alt="image" src="https://user-images.githubusercontent.com/32640567/216798640-d4100279-6c29-445c-b503-6f72e2cf3aed.png">
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/executor/RegionWriteExecutor.java:
##########
@@ -112,17 +116,21 @@ public static ConsensusWriteResponse fireTriggerAndInsert(
boolean hasFailedTriggerBeforeInsertion =
result.equals(TriggerFireResult.FAILED_NO_TERMINATION);
+ planNode.setMetricTime(System.nanoTime());
writeResponse = DataRegionConsensusImpl.getInstance().write(groupId, planNode);
// fire Trigger after the insertion
if (writeResponse.isSuccessful()) {
+ startTime = System.nanoTime();
result = visitor.process(planNode, TriggerEvent.AFTER_INSERT);
if (hasFailedTriggerBeforeInsertion || !result.equals(TriggerFireResult.SUCCESS)) {
TSStatus triggerError = new TSStatus(TSStatusCode.TRIGGER_FIRE_ERROR.getStatusCode());
triggerError.setMessage(
"Meet trigger error before/after the insertion, the insertion itself is completed.");
writeResponse = ConsensusWriteResponse.newBuilder().setStatus(triggerError).build();
}
+ PerformanceOverviewMetricsManager.getInstance()
Review Comment:
Why `consensus` as it has been collected in statemachine?
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/executor/RegionWriteExecutor.java:
##########
@@ -201,6 +209,7 @@ private RegionExecutionResult executeDataInsert(
// data insertion should be blocked by data deletion, especially when deleting timeseries
context.getRegionWriteValidationRWLock().readLock().lock();
try {
+ final long startTime = System.nanoTime();
Review Comment:
Move to the outside of the lock grab
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org