When there is only one healthy team, the bug will set a server's status as unhealthy;
which causes the healthyTeam to 0, triggering StorageServerTracker to loop back;
which resets the server's status to healthy, and thus the healthyTeam to non-zero.
This pattern will cause infinite loop.
Infinite loop will prevent TraceEvent from flushing, which causes
TraceEvent to use most of memory and out-of-memory.
Kudos to JingYu Zhou (jingyu_zhou@apple.com) who is the main contributor who found the bug!
This incorporates the change for defending DD from misconfigured
locality entries.
The check for misconfigured locality was in keyValueStoreTypeTracker,
but the storage engine switch PR moves the isCorrectDC checking out of
the tracker and move it into storageServerTracker
When a worker has incorrect locality, the worker will be excluded from
storage recruitment.
When the worker has its locality corrected by system operators,
the worker will be reincluded for storage recruitment.
When we add simulation test that misconfigure a cluster by not setting some
locality entries, we should set DD_VALIDATE_LOCALITY always true.
Otherwise, simulation tests may fail.
Due to randomness, when unhealthy teams are majority while there still
exists healthy teams, getTeam function may be unlucky to find
any feasible (ok) team, which leads to BestTeamStuck situation.
This commit increases the tries from 10 to 20.
A long-term solution may first find all feasible teams and choose a random
one from them. Since This can affect the statistics of which team is picked.
So it is not included in this commit.
Non-functional change: This commit removes unneeded printf introduced by
fast restore PR 1404.
1) Rely on the fact that the wrong storeType server will signal removeWrongStoreType
back to re-check the next server with wrong storeType.
Do not rely on timeout as a safeguard to trigger removeWrongStoreType actor.
Note:
removeWrongStoreType actor will not check if there is a storage server with wrong
store type, unless the DD is healthy.
Removing a storage server while DD is not healthy may trigger weird failure cases and
also cause negative impact on cluster performance.
2) Remove try catch in KeyValueStoreTypeTracker
Let the caller handle exceptions, e.g., actor cancelling exception
3) Cleanup debug message and clang-format code
If a cluster does not change its storeType for a while, we do not need to
call removeWrongStoreType actor periodically.
This solution is the same as how badTeamRemover actor is handled.
In case the wrong storeType SS picked to be removed fails before
it triggers the next round of checking if a SS has wrong store type,
we should time out and invoke the checking.
Otherwise, the removeWrongStoreType actor will never be running again.
Multiple storage server recruitment requests may be buffered in
cluster controller, hoping that in the near future cluster controller
will find an available worker for the request.
It is possible that many outstanding storage recruitment requests are
fullfilled by the cluster controller in a very short time interval.
When DD recruit those requests, it blindly initiaze a storage server
on the recruited worker and let the storage server tracker remove
storage servers on the same process (ip, port).
This is problematic because multiple SS on the same process can push
the process OOM. Even in simulation, initializing too many SS causes
simulator OOM.
This commit limits the max number of SS on a process to be 2.
We cannot enforce the number of SS on a process to be 1 right now,
because current simulation tests may change configuration in a situation that
without allowing more than 1 SS on a process will fail the tests.
If DD checks storage server's storeType before some storage servers are
fully available, DD may miss those storage servers to remove.
To ensure no storage servers with wrong storeType is missed, SS marks the
doRemoveWrongStoreType to be true.
To avoid removing multiple servers at the same time, the actor waits for a
configurable delay before checking and removing a storage server.
When a storage server is detected by the storageServerFailureTracker as failed,
we should not immediately restart recruiting new storage server on the same address,
which will cause OOM error on the address.
The existing logic removeServer() function will remove the failed server from
server_info and trigger recruiting new storage servers.
In the old logic of switching storage engines, it marks a storage server
with wrong store type as undesired even though this can lead to no healthy team.
In the first version of the new storage engine switch, we mimic the same logic
of the old version.
Trigger does not have an effect if the receiver is not waiting on the trigger.
To ensure the wrong store type server that is selected to be removed is removed,
we should use an AysncVar<bool> to trigger the storage tracker.
tclinken