[BugFix] Publish version operation should be successful even if the warehouse is not exist #52967
Conversation
…he warehouse is not exist Signed-off-by: yandongxiao <[email protected]>
yandongxiao
force-pushed
the
bugfix/publish-version-should-be-successful-2
branch
from
3043d02
to
e91a9ce
Compare
|
|
||
| // publish version operation should be successful even if the warehouse is not exist | ||
| if (!GlobalStateMgr.getCurrentState().getWarehouseMgr().warehouseExists(warehouseId)) { | ||
| warehouseId = WarehouseManager.DEFAULT_WAREHOUSE_ID; |
There was a problem hiding this comment.
log a message to this situation. ideally this should not happen.
Also need to consider another situation where the warehouse exists but there is no compute node in the warehouse, what to do then?
There was a problem hiding this comment.
In this situation, FE will fail, and user can solve this problem by add compute node.
But If the warehouse is deleted, the new created warehouse with the same name will have a different warehouse ID.
There was a problem hiding this comment.
why bother user to add nodes into warehouse, can we move one step forward, choose nodes in other warehouse until there is no warehouse to choose to accomplish this task and then report failure to the end user?
There was a problem hiding this comment.
Have logged a message to this situation.
There was a problem hiding this comment.
Currently, we have assumed that nodes exist in the default warehouse. As for the logic of selecting other warehouses, I think it needs to be done after removing the reliance on the default warehouse. Currently, other parts of the code also use the default warehouse as a backup.
There was a problem hiding this comment.
what if there is not possible to fix it in short time. Failing the ops in the beginning if possible but not in the last step. this publish txn can't be cancelled and will block all the subsequent operations in the queue.
There was a problem hiding this comment.
It is purely valid to have a warehouse but with no node. There is no such restriction that a warehouse must have nodes. If the situation can be solved with less assumption and it is reasonable to remove the assumption, then we should go without the assumption.
There was a problem hiding this comment.
what if there is not possible to fix it in short time. Failing the ops in the beginning if possible but not in the last step. this publish txn can't be cancelled and will block all the subsequent operations in the queue.
For public version operations, if no Node exists, it is reasonable to select the Node in the Default Warehouse.
There was a problem hiding this comment.
It is purely valid to have a warehouse but with no node. There is no such restriction that a warehouse must have nodes. If the situation can be solved with less assumption and it is reasonable to remove the assumption, then we should go without the assumption.
I think we can totally fix this in another PR.
There was a problem hiding this comment.
Forgot to use background warehouse, and I have fixed to use this warehouse.
And I also fix the situation where no alive node in user's warehouse.
Signed-off-by: yandongxiao <[email protected]>
Signed-off-by: yandongxiao <[email protected]>
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]❌ fail : 5 / 22 (22.73%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
Why I'm doing:
Publish version operation should be successful even if the warehouse is not exist
What I'm doing:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: