[Subtask]: Modify DefaultTableService to be compatible with master-slave mode.#3927
[Subtask]: Modify DefaultTableService to be compatible with master-slave mode.#3927xxubai merged 16 commits intoapache:masterfrom
Conversation
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions. |
|
Please perform CR on these implementations. |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
…lave mode is enabled. apache#3845
…on in master-slave mode. apache#3921
…on in master-slave mode. apache#3921
github-actions
Bot
added
the
type:docs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3927 +/- ##
============================================
- Coverage 29.90% 29.65% -0.26%
+ Complexity 4160 4155 -5
============================================
Files 670 667 -3
Lines 53533 53598 +65
Branches 6797 6797
============================================
- Hits 16008 15893 -115
- Misses 36343 36533 +190
+ Partials 1182 1172 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Create and start AmsAssignService for bucket assignment | ||
| // The factory will handle different HA types (ZK, database, etc.) | ||
| amsAssignService = new AmsAssignService(haContainer, serviceConfig); | ||
| bucketAssignStore = BucketAssignStoreFactory.create(haContainer, serviceConfig); |
There was a problem hiding this comment.
If I understand correctly, when BucketAssignStore initialization fails, the service does not fully fall back to non-master-slave behavior. initialize() falls back to loading all table runtimes because bucketAssignStore is null, but other paths still keep master-slave semantics, for example only the leader runs exploreTableRuntimes() and triggerTableAdded() may still return early without creating local runtimes. This can leave the in-memory table runtime state inconsistent with the expected runtime state derived from the database.
There was a problem hiding this comment.
There is indeed a risk associated with this bug; I will fix it.
4 participants
Why are the changes needed?
In DefaultTable Service, it is compatible with master-slave mode. In master-slave mode, it is necessary to load tables from catalog, determine the addition and deletion of tables, complete the allocation of tables to bucketId, and the logic of ams nodes loading the tables they are responsible for based on bucketId.
Close #3923 .
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation