Support having multiple crr locations configured in the queueProcessor #2632
Conversation
Hello kerkesni,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
... and 1 file with indirect coverage changes
@@ Coverage Diff @@
## development/9.1 #2632 +/- ##
===================================================
+ Coverage 73.08% 73.47% +0.39%
===================================================
Files 201 201
Lines 13413 13403 -10
===================================================
+ Hits 9803 9848 +45
+ Misses 3600 3545 -55
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Kerkesni
changed the title
Kerkesni
force-pushed
the
improvement/BB-662
branch
3 times, most recently
from
53c9fcd to
09fd95f
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
|
The comments point to a previous run of codecov which is weird, but i added tests explicitly for those cases so they should be covered. |
Kerkesni
force-pushed
the
improvement/BB-662
branch
4 times, most recently
from
8ae0788 to
622f266
Compare
| replicationEndpoint: this.bootstrapList.find(endpoint => endpoint.site === site), | ||
| }; |
There was a problem hiding this comment.
probably not such a big deal today with limited number of locations, but could become one: we are looping through the whole list every time..... even though actually we are actually already looping over every site (looking at task.js)
const bootstrapList = config.getBootstrapList();
const siteNames = bootstrapList.map(i => i.site);
// initialize per site destination configs
const destConfig = {};
siteNames.forEach(site => {
destConfig[site] = config.getReplicationSiteDestConfig(site);
});
probably not something we can change in the API, but when loading the config is could do some reprocessing to just linearly build just once both the list of sites and replicationEndpoints.
Kerkesni
force-pushed
the
improvement/BB-662
branch
from
622f266 to
6a787b4
Compare
QueueProcessor currently sets the destination host to the host of the last crr site in the bootstrap list. This works in the 7.x branch as we filter the bootstrap list to only contain the relevant site. This filtering was done in the entrypoint of the QueueProcessor (i.e task.js) which changed in 8.x and 9.x. Issue: BB-662
Kerkesni
force-pushed
the
improvement/BB-662
branch
from
6a787b4 to
b11a9f9
Compare
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue BB-662. Goodbye kerkesni. |
QueueProcessor currently sets the destination host to the host of the last crr site in the bootstrap list, so when having multiple crr locations configured this will not work.
Issue: BB-662