fix(replica): fix incorrect error code when secondary replica disk status is abnormal

[limowang]

Fix #2386

When the primary replica’s disk status is normal but the secondary replica encounters disk issues (IO_ERROR or SPACE_INSUFFICIENT), the primary correctly rejects write requests. However, the Java client logs a misleading error message: “The request maybe too large!” instead of the actual disk error, which makes troubleshooting more difficult.

Root Cause

Bug 1: When the primary replica’s disk status is normal but the secondary disk is abnormal, C++ returns ERR_OK.

Location: src/replica/replica_2pc.cpp:184-188

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));
// BUG: When secondary is abnormal but primary is NORMAL, this returns ERR_OK!
return;
}

Bug 2: Java maps empty response to ERR_INVALID_DATA

Location: java-client/src/main/java/org/apache/pegasus/rpc/async/ThriftFrameDecoder.java:70-81

When C++ returns ERR_OK with no response body (because the write request was rejected), the Java client's recv_data() throws a TException, which is then incorrectly mapped to ERR_INVALID_DATA, resulting in misleading log output.

Solution:

1. Modified replica_2pc.cpp to return more accurate error codes:

   • When the primary node’s disk has issues, it returns the corresponding disk status code.
   • When the primary node’s disk is normal but the secondary replica’s disk is abnormal (ERR_DISK_IO_ERROR or ERR_DISK_INSUFFICIENT), it no longer incorrectly returns ERR_OK, but instead returns the corresponding disk error code of the secondary replica node.

2. Updated the Java client to handle ERR_DISK_IO_ERROR error codes.

[empiredan]

@limowang Thank you very much for helping fix this issue! Please add more precise details to the description, including what the symptoms of the bug are, what the root cause analysis shows, and what fixes were made. This information will later be included in the commit message, so please ensure it is accurate.

[empiredan]

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.

[limowang]

Thank you very much for your suggestion. I have introduced a more precise error code, ERR_SECONDARY_DISK_ABNORMAL, to indicate that the disk on the node hosting the secondary replica is abnormal (either insufficient disk space or an I/O error).

[empiredan]

I tend to prefer returning ERR_DISK_INSUFFICIENT or ERR_DISK_IO_ERROR for disk issues on secondary replicas as well. The reasons are as follows:

1. Whether primary or secondary, they are essentially replicas of the data. From a high-level perspective, any disk issue on a replica results in a failure to write data.
2. A two-dimensional classification is introduced if disk issues on primary and secondary replicas are represented by different error codes, which will bloat the error code system.

[limowang]

Your suggestion is very reasonable, and I will make the changes accordingly.

[empiredan]

Suggested change

	if (FLAGS_reject_write_when_disk_insufficient &&
	(_dir_node->status != disk_status::NORMAL || _primary_states.secondary_disk_abnormal())) {
	if (FLAGS_reject_write_when_disk_insufficient) {

[empiredan]

Suggested change

	for (const auto &kv : _primary_states.secondary_disk_status) {
	if (kv.second != disk_status::NORMAL) {
	response_client_write(request, disk_status_to_error_code(kv.second));
	break;
	}
	}
	for (const auto &[addr, secondary_status] : _primary_states.secondary_disk_status) {
	if (secondary_status != disk_status::NORMAL) {
	LOG_INFO("partition[{}] secondary[{}] disk space is {}",
	_primary_states.pc.pid,
	addr,
	enum_to_string(secondary_status));
	response_client_write(request, disk_status_to_error_code(secondary_status));
	return;
	}
	}

[empiredan]

Suggested change

return;

[empiredan]

Suggested change

	response_client_write(request, disk_status_to_error_code(_dir_node->status));
	response_client_write(request, disk_status_to_error_code(_dir_node->status));
	return;