Envoy: Multiple Calls To OnCodecLowLevelReset For Stream

In the realm of Envoy proxy, a high-performance proxy designed for cloud-native applications, the onCodecLowLevelReset function plays a crucial role in managing stream resets. However, a recent issue has surfaced where this function is being called multiple times for the same stream, particularly after the implementation of pull request #37784. This can lead to unexpected behavior and potential problems within the system. Understanding why this happens and how to address it is vital for maintaining the stability and reliability of Envoy-based applications.

The Issue: Multiple onCodecLowLevelReset Calls

The core problem lies in the fact that the onCodecLowLevelReset function, which is intended to be called once when a stream is reset at a low level, is being triggered multiple times under certain conditions. Specifically, this occurs when a non-zero error code is encountered during the stream reset process. This can lead to redundant operations and potentially disrupt the normal flow of data, making it essential to diagnose and resolve this behavior.

To fully grasp the issue, let's examine the two specific instances where onCodecLowLevelReset is being called:

First Instance: ConnectionImpl::StreamImpl::resetStreamWorker

The initial call to onCodecLowLevelReset occurs within the resetStreamWorker function. This function is responsible for handling stream resets within the Envoy connection implementation. The relevant code snippet is as follows:

void ConnectionImpl::StreamImpl::resetStreamWorker(StreamResetReason reason) {
  if (stream_id_ == -1) {
    // Handle the case where client streams are reset before headers are created.
    // For example, if we send local reply after the stream is created but before
    // headers are sent, we will end up here.
    ENVOY_CONN_LOG(trace, "Stream {} reset before headers sent.", parent_.connection_, stream_id_);
    Status status = parent_.onStreamClose(this, 0);
    ASSERT(status.ok());
    return;
  }
  if (codec_callbacks_) {
    codec_callbacks_->onCodecLowLevelReset();
  }
  parent_.adapter_->SubmitRst(stream_id_,
                              static_cast<http2::adapter::Http2ErrorCode>(reasonToReset(reason)));
}

In this scenario, if the codec_callbacks_ exist, the onCodecLowLevelReset function is invoked. This is a standard part of the stream reset process, ensuring that the codec is notified of the reset event.

Second Instance: onResetEncoded

The second call to onCodecLowLevelReset occurs within the onResetEncoded function. This function is triggered when a reset is encoded, and it includes a check for a non-zero error code. Here’s the relevant code:

void onResetEncoded(uint32_t error_code) {
  if (codec_callbacks_ && error_code != 0) {
    codec_callbacks_->onCodecLowLevelReset();
  }
}

This is where the problem arises. If a non-zero error_code is present, onCodecLowLevelReset is called again. This duplicate call is unnecessary and can lead to issues in how the system handles stream resets. The key here is the condition error_code != 0, which triggers the call when an error is present.

Why is This a Problem?

The multiple calls to onCodecLowLevelReset can lead to several potential issues:

1. Redundant Operations: The function might trigger the same cleanup or reset operations multiple times, which is inefficient and can consume unnecessary resources.
2. Race Conditions: If onCodecLowLevelReset involves complex state changes, multiple calls could lead to race conditions, where the order of execution affects the final state of the system.
3. Unexpected Behavior: In some cases, the duplicate calls might lead to unexpected behavior, such as incorrect metrics, logging errors, or even crashes, depending on the specific implementation and context.

Addressing the Issue

To resolve the issue of multiple onCodecLowLevelReset calls, a fix is needed within the Envoy codebase. The most straightforward solution is to prevent the redundant call in the onResetEncoded function. This can be achieved by adding a check to ensure that onCodecLowLevelReset has not already been called for the stream.

One potential approach involves introducing a flag or state variable that indicates whether onCodecLowLevelReset has been called. This flag can be set in the resetStreamWorker function and checked in the onResetEncoded function. If the flag is already set, the call to onCodecLowLevelReset in onResetEncoded can be skipped.

Here’s a conceptual example of how this might be implemented:

1. Add a boolean flag, such as low_level_reset_called_, to the StreamImpl class.
2. In the resetStreamWorker function, set low_level_reset_called_ to true after calling codec_callbacks_->onCodecLowLevelReset().
3. In the onResetEncoded function, check the value of low_level_reset_called_ before calling codec_callbacks_->onCodecLowLevelReset(). If it’s true, skip the call.

This approach ensures that onCodecLowLevelReset is called only once per stream reset, preventing the issues associated with multiple calls.

Potential Break Change for Google

The issue also raises a concern about potential break changes, particularly for Google, which heavily relies on Envoy in its infrastructure. A break change refers to a modification in a system that may cause existing functionalities or integrations to fail or behave unexpectedly. In the context of Envoy, a break change could impact services and applications that depend on the proxy's behavior.

To mitigate the risk of break changes, it's essential to carefully evaluate the impact of any proposed fix. This includes thoroughly testing the fix in various scenarios, considering backward compatibility, and providing clear communication to users about the changes.

Evaluation and Testing

Before deploying any fix, it's crucial to conduct comprehensive testing. This testing should cover both unit tests and integration tests to ensure that the fix resolves the issue without introducing new problems. Unit tests can verify the behavior of individual components, while integration tests can assess the overall system behavior.

Backward Compatibility

When implementing a fix, it's important to consider backward compatibility. This means ensuring that the fix doesn't break existing functionalities or integrations. In some cases, this might involve introducing configuration options or flags that allow users to opt-in to the new behavior or maintain the old behavior if necessary.

Communication and Documentation

Clear communication is essential when introducing changes that might have an impact on users. This includes providing detailed release notes, updating documentation, and communicating the changes through appropriate channels, such as mailing lists or forums. By keeping users informed, it's possible to minimize the disruption caused by break changes.

Conclusion

The issue of multiple calls to onCodecLowLevelReset in Envoy highlights the complexities of managing stream resets in a high-performance proxy. By understanding the root cause of the problem and implementing appropriate fixes, it's possible to ensure the stability and reliability of Envoy-based applications. The proposed solution, which involves preventing redundant calls in the onResetEncoded function, can effectively address the issue. However, it's crucial to carefully evaluate the impact of any fix, conduct thorough testing, and provide clear communication to users to minimize the risk of break changes. Addressing this issue is a crucial step in ensuring the continued robustness and performance of Envoy as a critical component of modern cloud-native infrastructure.

For further reading on Envoy Proxy and its functionalities, you can visit the official Envoy Proxy documentation. This resource provides comprehensive information about Envoy's architecture, features, and configuration options.