Fix acceleration in mirror-3-dc groups #7931
Conversation
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| @@ -236,4 +235,45 @@ inline bool RestoreWholeFromMirror(TBlobState& state) { | |||
| return !state.Whole.NotHere(); | |||
| } | |||
|
|
|||
| inline ui32 MakeSlowSubgroupDiskMaskForTwoSlowest(TBlobState &state, const TBlobStorageGroupInfo &info, | |||
There was a problem hiding this comment.
Are you sure it should be inline?
| @@ -166,7 +165,7 @@ class IStrategy { | |||
|
|
|||
| struct TBlackboard { | |||
| enum EAccelerationMode { | |||
| AccelerationModeSkipOneSlowest, | |||
| AccelerationModeSkipTwoSlowest, | |||
There was a problem hiding this comment.
And what if we want to skip one slowest?
| return 0; | ||
| } | ||
|
|
||
| for (size_t idx = 0; idx < state.Disks.size(); ++idx) { |
| } | ||
| } | ||
|
|
||
| return slowDiskSubgroupMask; |
There was a problem hiding this comment.
Honestly I don't like two parallel sets of slow disks -- IsSlow flag and this mask. They will eventually go out of sync sometimes after a code change.
| @@ -154,7 +154,7 @@ ui64 TGetImpl::GetTimeToAccelerateNs(TLogContext &logCtx, NKikimrBlobStorage::EV | |||
| *Info, *Blackboard.GroupQueues, queueId, &worstDisks, | |||
| AccelerationParams.PredictedDelayMultiplier); | |||
| } | |||
| return worstDisks[std::min(3u, (ui32)worstDisks.size() - 1)].PredictedNs; | |||
| return worstDisks[std::min(2u, (ui32)worstDisks.size() - 1)].PredictedNs; | |||
There was a problem hiding this comment.
No numeric constants except for 0 and 1, please :)
| @@ -45,7 +45,7 @@ class TAcceleratePut3dcStrategy : public TStrategyBase { | |||
| } | |||
| } | |||
| if (badDiskMask > 0) { | |||
| // Mark the 'bad' disk as the single slow disk | |||
| // Mark all the slow disks | |||
| for (size_t diskIdx = 0; diskIdx < state.Disks.size(); ++diskIdx) { | |||
| state.Disks[diskIdx].IsSlow = badDiskMask & (1 << diskIdx); | |||
There was a problem hiding this comment.
Possible out of sync goes here.
| @@ -363,10 +363,11 @@ void TStrategyBase::Evaluate3dcSituation(const TBlobState &state, | |||
| } | |||
| } | |||
|
|
|||
| void TStrategyBase::Prepare3dcPartPlacement(const TBlobState &state, | |||
| bool TStrategyBase::Prepare3dcPartPlacement(const TBlobState &state, | |||
There was a problem hiding this comment.
Return fullPlacement through out parameter. Otherwise there is a mixup of return values and return parameters -- it is not obvious what this function returns (based on its name).
|
|
||
| return slowDiskSubgroupMask; | ||
| case TBlackboard::AccelerationModeSkipTwoSlowest: { | ||
| return MakeSlowSubgroupDiskMaskForTwoSlowest(state, info, blackboard, isPut, accelerationParams); |
There was a problem hiding this comment.
I suggest you making this code a bit more universal -- mark slowest N disks. Instead of patching the code every time.
| @@ -71,30 +71,11 @@ namespace NKikimr { | |||
| // issue request for a specific disk; returns true if the request was issued and not yet completed, otherwise | |||
| // false | |||
|
|
|||
| if (info.GetTotalVDisksNum() > 1) { | |||
| if (info.GetTotalVDisksNum() >= 3) { | |||
There was a problem hiding this comment.
This is mirror-3-dc-specific strategy, TotalVDisksNum can't be less than 9.
… into bs/dsproxy/fix-accelerates
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
serbel324
force-pushed
the
bs/dsproxy/fix-accelerates
branch
from
1971937 to
46f9726
Compare
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
Fixed Acceleration in mirror-3-dc and UT environment
Changelog category
Additional information
...