empiredan commented on code in PR #2387:
URL:
https://github.com/apache/incubator-pegasus/pull/2387#discussion_r2945309101
##########
src/replica/replica_2pc.cpp:
##########
@@ -183,7 +183,13 @@ void replica::on_client_write(dsn::message_ex *request,
bool ignore_throttling)
if (FLAGS_reject_write_when_disk_insufficient &&
(_dir_node->status != disk_status::NORMAL ||
_primary_states.secondary_disk_abnormal())) {
- response_client_write(request,
disk_status_to_error_code(_dir_node->status));
+ if (_dir_node->status != disk_status::NORMAL) {
+ // Primary replica disk is abnormal, return the corresponding
error code
+ response_client_write(request,
disk_status_to_error_code(_dir_node->status));
+ } else {
+ // Secondary replica disk is abnormal but primary is OK
+ response_client_write(request, ERR_REPLICATION_FAILURE);
Review Comment:
The `ERR_REPLICATION_FAILURE` error code appears to have been reserved early
on, and no history of it being used has been found. The reason for using
`ERR_REPLICATION_FAILURE` seems to be simply to reuse an existing error code to
indicate that the issue occurred during replication to a secondary replica?
As I understand, your original intention was to use
`ERR_REPLICATION_FAILURE` to distinguish whether the problem is on the primary
or the secondary. However, based on the code, the error here is quite clear—it
is either a disk issue on the primary or on the secondary. In fact, the
replication to the secondary has not even happened yet.
For faster issue diagnosis, would it make sense to replace it with
disk-related error codes such as `ERR_DISK_INSUFFICIENT` and
`ERR_DISK_IO_ERROR`? This way, we can quickly identify that a machine in the
cluster has a disk problem—either running out of space or encountering I/O
errors—which can typically be detected quickly through monitoring metrics.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
