You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/09 13:04:52 UTC
[GitHub] [doris] weizuo93 opened a new pull request, #10721: [Bug] Fix bug that segments are removed as trash but tablet meta is normal
weizuo93 opened a new pull request, #10721:
URL: https://github.com/apache/doris/pull/10721
# Proposed changes
Issue Number: close #10720
## Problem Summary:
We replaced the original disk on the BE node with a new disk by wrong operation when BE restarted. When we discovered the mistake and added the original disk. There is no unhealthy replica in the cluster after a period of time, and we removed the wrong disk. When query comes, exception information is thrown and the error code is `-3109` which means `failed to open segment`. We found that segment files for some tablets had been removed as trash but tablet meta is normal in original disk. These abnormal tablets can not be detected and repaired by FE.
## Checklist(Required)
1. Does it affect the original behavior: (No)
2. Has unit tests been added: (No Need)
3. Has document been added or modified: (No Need)
4. Does it need to update dependencies: (No)
5. Are there any changes that cannot be rolled back: (No)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10721: [Bug](tablet) Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10721:
URL: https://github.com/apache/doris/pull/10721#issuecomment-1182702324
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #10721: [Bug] Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10721:
URL: https://github.com/apache/doris/pull/10721#discussion_r917331120
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -154,13 +154,24 @@ Status TabletManager::_add_tablet_unlocked(TTabletId tablet_id, const TabletShar
// During restore process, snapshot loader
// replaced the old tablet's rowset with new rowsets, but the tablet path is reused, if drop files
// here, the new rowset's file will also be dropped, so use keep files here
- bool keep_files = force ? true : false;
+ bool keep_files = force;
if (force ||
(new_version > old_version || (new_version == old_version && new_time > old_time))) {
// check if new tablet's meta is in store and add new tablet's meta to meta store
res = _add_tablet_to_map_unlocked(tablet_id, tablet, update_meta, keep_files,
true /*drop_old*/);
} else {
+ if (!keep_files) {
+ tablet->set_tablet_state(TABLET_SHUTDOWN);
Review Comment:
I am confused that, this is a "add tablet" method, but why we finally set tablet to `TABLET_SHUTDOWN`?
Should it be `existed_tablet`?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizuo93 merged pull request #10721: [Bug](tablet) Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
weizuo93 merged PR #10721:
URL: https://github.com/apache/doris/pull/10721
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizuo93 commented on a diff in pull request #10721: [Bug] Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10721:
URL: https://github.com/apache/doris/pull/10721#discussion_r918485330
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -154,13 +154,24 @@ Status TabletManager::_add_tablet_unlocked(TTabletId tablet_id, const TabletShar
// During restore process, snapshot loader
// replaced the old tablet's rowset with new rowsets, but the tablet path is reused, if drop files
// here, the new rowset's file will also be dropped, so use keep files here
- bool keep_files = force ? true : false;
+ bool keep_files = force;
if (force ||
(new_version > old_version || (new_version == old_version && new_time > old_time))) {
// check if new tablet's meta is in store and add new tablet's meta to meta store
res = _add_tablet_to_map_unlocked(tablet_id, tablet, update_meta, keep_files,
true /*drop_old*/);
} else {
+ if (!keep_files) {
Review Comment:
> keep_files must be false here.
OKļ¼Thank you.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10721: [Bug](tablet) Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10721:
URL: https://github.com/apache/doris/pull/10721#issuecomment-1182702307
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizuo93 commented on a diff in pull request #10721: [Bug] Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10721:
URL: https://github.com/apache/doris/pull/10721#discussion_r917337271
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -154,13 +154,24 @@ Status TabletManager::_add_tablet_unlocked(TTabletId tablet_id, const TabletShar
// During restore process, snapshot loader
// replaced the old tablet's rowset with new rowsets, but the tablet path is reused, if drop files
// here, the new rowset's file will also be dropped, so use keep files here
- bool keep_files = force ? true : false;
+ bool keep_files = force;
if (force ||
(new_version > old_version || (new_version == old_version && new_time > old_time))) {
// check if new tablet's meta is in store and add new tablet's meta to meta store
res = _add_tablet_to_map_unlocked(tablet_id, tablet, update_meta, keep_files,
true /*drop_old*/);
} else {
+ if (!keep_files) {
+ tablet->set_tablet_state(TABLET_SHUTDOWN);
Review Comment:
> I am confused that, this is a "add tablet" method, but why we finally set tablet to `TABLET_SHUTDOWN`? Should it be `existed_tablet`?
@morningman
Yes, it may be `existed_tablet` due to misoperation of the disk. You can refer to this issue #10720
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on a diff in pull request #10721: [Bug] Fix bug that segments are removed as trash but tablet meta is normal
Posted by GitBox <gi...@apache.org>.
dataroaring commented on code in PR #10721:
URL: https://github.com/apache/doris/pull/10721#discussion_r918062727
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -154,13 +154,24 @@ Status TabletManager::_add_tablet_unlocked(TTabletId tablet_id, const TabletShar
// During restore process, snapshot loader
// replaced the old tablet's rowset with new rowsets, but the tablet path is reused, if drop files
// here, the new rowset's file will also be dropped, so use keep files here
- bool keep_files = force ? true : false;
+ bool keep_files = force;
if (force ||
(new_version > old_version || (new_version == old_version && new_time > old_time))) {
// check if new tablet's meta is in store and add new tablet's meta to meta store
res = _add_tablet_to_map_unlocked(tablet_id, tablet, update_meta, keep_files,
true /*drop_old*/);
} else {
+ if (!keep_files) {
Review Comment:
keep_files must be false here.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org