You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "DanielWang2035 (via GitHub)" <gi...@apache.org> on 2025/01/17 06:09:47 UTC
[PR] Load: clean half-written TsFiles when retrying first phase [iotdb]
DanielWang2035 opened a new pull request, #14717:
URL: https://github.com/apache/iotdb/pull/14717
(no 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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "DanielWang2035 (via GitHub)" <gi...@apache.org>.
DanielWang2035 commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952252879
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/load/converter/LoadTreeStatementDataTypeConvertExecutionVisitor.java:
##########
@@ -109,6 +109,10 @@ file, new IoTDBTreePattern(null), Long.MIN_VALUE, Long.MAX_VALUE, null, null)) {
|| result.getCode() == TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode()
|| result.getCode()
== TSStatusCode.LOAD_IDEMPOTENT_CONFLICT_EXCEPTION.getStatusCode())) {
+ LOGGER.warn(
Review Comment:
table?
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu merged PR #14717:
URL: https://github.com/apache/iotdb/pull/14717
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "DanielWang2035 (via GitHub)" <gi...@apache.org>.
DanielWang2035 commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952240421
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/scheduler/load/LoadTsFileScheduler.java:
##########
@@ -273,6 +272,7 @@ public void start() {
final long startTime = System.nanoTime();
try {
// if failed to load some TsFiles, then try to convert the TsFiles to Tablets
+ LOGGER.info("Load TsFile failed, will try to convert to tablets and insert.");
Review Comment:
which tsfile
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "DanielWang2035 (via GitHub)" <gi...@apache.org>.
DanielWang2035 commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952247230
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java:
##########
@@ -908,6 +908,16 @@ public TSStatus writeLoadTsFileNode(
return status;
}
+ final DataRegion dataRegion = getDataRegion(dataRegionId);
Review Comment:
concurrent
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "DanielWang2035 (via GitHub)" <gi...@apache.org>.
DanielWang2035 commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952247230
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java:
##########
@@ -908,6 +908,16 @@ public TSStatus writeLoadTsFileNode(
return status;
}
+ final DataRegion dataRegion = getDataRegion(dataRegionId);
Review Comment:
concurrent
try catch
error -> warn
after ratelimiter
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952451966
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/scheduler/load/LoadTsFileScheduler.java:
##########
@@ -270,9 +269,18 @@ public void start() {
if (isLoadSuccess) {
stateMachine.transitionToFinished();
} else {
+ final StringBuilder failedTsFiles = new StringBuilder();
+ for (int i : failedTsFileNodeIndexes) {
+ failedTsFiles
+ .append(tsFileNodeList.get(i).getTsFileResource().getTsFilePath())
+ .append(", ");
+ }
final long startTime = System.nanoTime();
try {
// if failed to load some TsFiles, then try to convert the TsFiles to Tablets
+ LOGGER.info(
+ "Load TsFile failed, will try to convert to tablets and insert. Failed TsFiles: [{}]",
Review Comment:
```suggestion
"Load TsFile(S) failed, will try to convert to tablets and insert. Failed TsFiles: {}",
```
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "DanielWang2035 (via GitHub)" <gi...@apache.org>.
DanielWang2035 commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952251945
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java:
##########
@@ -936,26 +946,18 @@ public TSStatus executeLoadCommand(
try {
switch (loadCommand) {
case EXECUTE:
- if (loadTsFileManager.loadAll(uuid, isGeneratedByPipe, progressIndex)) {
- status = RpcUtils.SUCCESS_STATUS;
- } else {
- status.setCode(TSStatusCode.LOAD_FILE_ERROR.getStatusCode());
- status.setMessage(
- String.format(
- "No load TsFile uuid %s recorded for execute load command %s.",
- uuid, loadCommand));
+ if (!loadTsFileManager.loadAll(uuid, isGeneratedByPipe, progressIndex)) {
Review Comment:
revert
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952454235
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java:
##########
@@ -910,17 +910,36 @@ public TSStatus writeLoadTsFileNode(
LoadTsFileRateLimiter.getInstance().acquire(pieceNode.getDataSize());
+ final DataRegion dataRegion = getDataRegion(dataRegionId);
+ if (dataRegion == null) {
+ LOGGER.warn(
+ "DataRegion {} not found on this DataNode when writing piece node"
+ + "of TsFile {} (maybe due to region migration), will skip.",
+ dataRegionId,
+ pieceNode.getTsFile());
+ return RpcUtils.SUCCESS_STATUS;
+ }
+
try {
- loadTsFileManager.writeToDataRegion(getDataRegion(dataRegionId), pieceNode, uuid);
+ loadTsFileManager.writeToDataRegion(dataRegion, pieceNode, uuid);
} catch (IOException e) {
- LOGGER.error(
+ LOGGER.warn(
"IO error when writing piece node of TsFile {} to DataRegion {}.",
pieceNode.getTsFile(),
dataRegionId,
e);
status.setCode(TSStatusCode.LOAD_FILE_ERROR.getStatusCode());
status.setMessage(e.getMessage());
return status;
+ } catch (Exception e) {
+ LOGGER.warn(
+ "Exception occurred when writing piece node of TsFile {} to DataRegion {}.",
+ pieceNode.getTsFile(),
+ dataRegionId,
+ e);
+ status.setCode(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode());
Review Comment:
```suggestion
status.setCode(TSStatusCode.LOAD_FILE_ERROR.getStatusCode());
```
--
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
Re: [PR] Load: convert to insert tablet on region replica set changes [iotdb]
Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu commented on code in PR #14717:
URL: https://github.com/apache/iotdb/pull/14717#discussion_r1952451966
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/scheduler/load/LoadTsFileScheduler.java:
##########
@@ -270,9 +269,18 @@ public void start() {
if (isLoadSuccess) {
stateMachine.transitionToFinished();
} else {
+ final StringBuilder failedTsFiles = new StringBuilder();
+ for (int i : failedTsFileNodeIndexes) {
+ failedTsFiles
+ .append(tsFileNodeList.get(i).getTsFileResource().getTsFilePath())
+ .append(", ");
+ }
final long startTime = System.nanoTime();
try {
// if failed to load some TsFiles, then try to convert the TsFiles to Tablets
+ LOGGER.info(
+ "Load TsFile failed, will try to convert to tablets and insert. Failed TsFiles: [{}]",
Review Comment:
```suggestion
"Load TsFile(s) failed, will try to convert to tablets and insert. Failed TsFiles: {}",
```
--
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