Re: [PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
SteveYurongSu merged PR #11641: URL: https://github.com/apache/iotdb/pull/11641 -- 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-unsubscr...@iotdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
VGalaxies commented on code in PR #11641:
URL: https://github.com/apache/iotdb/pull/11641#discussion_r1419936716
##
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/task/PipeTaskCoordinator.java:
##
@@ -118,11 +120,18 @@ public TSStatus startPipe(String pipeName) {
public TSStatus stopPipe(String pipeName) {
final boolean isStoppedByRuntimeException =
pipeTaskInfo.isStoppedByRuntimeException(pipeName);
final TSStatus status =
configManager.getProcedureManager().stopPipe(pipeName);
-if (status == RpcUtils.SUCCESS_STATUS && isStoppedByRuntimeException) {
- LOGGER.info(
- "Pipe {} has stopped successfully manually, stop its auto restart
process.", pipeName);
- pipeTaskInfo.setIsStoppedByRuntimeExceptionToFalse(pipeName);
- configManager.getProcedureManager().pipeHandleMetaChange(true, true);
+if (status == RpcUtils.SUCCESS_STATUS) {
+ if (isStoppedByRuntimeException) {
Review Comment:
No need, if modified in this way, the logic under this branch becomes
meaningless 😖
##
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/task/PipeTaskCoordinator.java:
##
@@ -118,11 +120,18 @@ public TSStatus startPipe(String pipeName) {
public TSStatus stopPipe(String pipeName) {
final boolean isStoppedByRuntimeException =
pipeTaskInfo.isStoppedByRuntimeException(pipeName);
final TSStatus status =
configManager.getProcedureManager().stopPipe(pipeName);
-if (status == RpcUtils.SUCCESS_STATUS && isStoppedByRuntimeException) {
- LOGGER.info(
- "Pipe {} has stopped successfully manually, stop its auto restart
process.", pipeName);
- pipeTaskInfo.setIsStoppedByRuntimeExceptionToFalse(pipeName);
- configManager.getProcedureManager().pipeHandleMetaChange(true, true);
+if (status == RpcUtils.SUCCESS_STATUS) {
+ if (isStoppedByRuntimeException) {
Review Comment:
No need, if modified in this way, the logic under this branch becomes
meaningless 😖
--
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-unsubscr...@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
SteveYurongSu commented on code in PR #11641:
URL: https://github.com/apache/iotdb/pull/11641#discussion_r1419924550
##
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/task/PipeTaskCoordinator.java:
##
@@ -118,11 +120,18 @@ public TSStatus startPipe(String pipeName) {
public TSStatus stopPipe(String pipeName) {
final boolean isStoppedByRuntimeException =
pipeTaskInfo.isStoppedByRuntimeException(pipeName);
final TSStatus status =
configManager.getProcedureManager().stopPipe(pipeName);
-if (status == RpcUtils.SUCCESS_STATUS && isStoppedByRuntimeException) {
- LOGGER.info(
- "Pipe {} has stopped successfully manually, stop its auto restart
process.", pipeName);
- pipeTaskInfo.setIsStoppedByRuntimeExceptionToFalse(pipeName);
- configManager.getProcedureManager().pipeHandleMetaChange(true, true);
+if (status == RpcUtils.SUCCESS_STATUS) {
+ if (isStoppedByRuntimeException) {
Review Comment:
```suggestion
if (!isStoppedByRuntimeException) {
```
--
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-unsubscr...@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
VGalaxies commented on PR #11641: URL: https://github.com/apache/iotdb/pull/11641#issuecomment-1836119257 The `needPushPipeMetaToDataNodes` parameter of `PipeHandleMetaChangeProcedure` is currently always set to false. -- 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-unsubscr...@iotdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
VGalaxies commented on PR #11641: URL: https://github.com/apache/iotdb/pull/11641#issuecomment-1831871746 Another question: When a pipe task encounters an exception, why is it necessary to rely on `lastEvent` instead of directly calling the `org.apache.iotdb.db.pipe.agent.runtime.PipeRuntimeAgent#report` method? See https://github.com/apache/iotdb/blob/master/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/task/subtask/PipeSubtask.java#L182 -- 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-unsubscr...@iotdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Pipe: avoid useless pipe meta sync (stopped status) to DN [iotdb]
VGalaxies opened a new pull request, #11641: URL: https://github.com/apache/iotdb/pull/11641 ## Description After #11279, when a pipe encounters a critical exception on DN, it stops all DR's pipe tasks related to that pipe and modifies the status of the relevant pipe to exception stopped on this DN. Subsequently, the CN collects this information through the heartbeat mechanism and pushes the changes to all DNs. This causes other DNs with running normal pipe tasks to be auto-restarted unnecessarily. This PR addresses this issue. From now on, there will be a subtle change in the semantics of the CN-side pipe status. When the status is exception stopped, it indicates that the pipe tasks on some (not necessarily all) DRs (DNs) of this pipe encountered a critical exception. This PR has: - [x] been self-reviewed. - [ ] concurrent read - [ ] concurrent write - [ ] concurrent read and write - [ ] added documentation for new or modified features or behaviors. - [ ] added Javadocs for most classes and all non-trivial methods. - [ ] added or updated version, __license__, or notice information - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage. - [ ] added integration tests. - [ ] been tested in a test IoTDB cluster. # Key changed/added classes (or packages if there are too many classes) in this PR -- 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-unsubscr...@iotdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
