morningman commented on a change in pull request #2781: Batch Operate Rollup
table in Doris #2671
URL: https://github.com/apache/incubator-doris/pull/2781#discussion_r376711463
##########
File path: fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
##########
@@ -830,8 +1008,14 @@ public void cancel(CancelStmt stmt) throws DdlException {
}
// find from new alter jobs first
- rollupJobV2 = getUnfinishedAlterJobV2(olapTable.getId());
- if (rollupJobV2 == null) {
+ if (cancelAlterTableStmt.getAlterJobIdList() != null) {
+ for (Long jobId : cancelAlterTableStmt.getAlterJobIdList()) {
+ rollupJobV2List.add(getUnfinishedAlterJobV2ByJobId(jobId));
Review comment:
`getUnfinishedAlterJobV2ByJobId()` may return null if job does not exist, so
`rollupJobV2List` may contains `null`. You didn't check it when using it after.
Another problem is, batch cancel operation can be partial success in your
implementation.
Although I advocate all operations to avoid partial success. But for cancel
operation, this change may be slightly more complicated. A two-phase protocal
may be needed to achieve this.
So if you keep the current implementation. The first requirement is to allow
the user to specify a non-existing job id to cancel. Second, the document needs
to indicate that there may be partial success here, and the user needs to retry
multiple times until a successful message is returned.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
