Skip to content

Revert "ext_proc: Ext proc half close on destroy and defer reset till trailers received. (#37083)"#37639

Merged
yanavlasov merged 1 commit intoenvoyproxy:mainfrom
yanavlasov:revert-pr-37083
Dec 20, 2024
Merged

Revert "ext_proc: Ext proc half close on destroy and defer reset till trailers received. (#37083)"#37639
yanavlasov merged 1 commit intoenvoyproxy:mainfrom
yanavlasov:revert-pr-37083

Conversation

@yanavlasov
Copy link
Copy Markdown
Contributor

@yanavlasov yanavlasov commented Dec 12, 2024

Additional Description:
This PR is suspected of causing instability in prod. The cause is not yet fully diagnosed, but it is reverted as a safety measure.

Risk Level: Low
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

@yanavlasov
Revert "ext_proc: Ext proc half close on destroy and defer reset till…
6c9e55e 
… trailers received. (envoyproxy#37083)"

This reverts commit 0b90f64.

Signed-off-by: Yan Avlasov <[email protected]>
@LinMoskovitch
Copy link
Copy Markdown

LinMoskovitch commented Dec 15, 2024

Hi @yanavlasov,

I noticed that you reverted PR #37083 due to suspected instability in production. I’ve started seeing UNKNOWN gRPC errors and no_healthy_upstream logs in Envoy.

Before this upgrade, even when there were upstream issues, I didn’t encounter UNKNOWN gRPC status codes. I suspect that the stricter enforcement of trailers in PR #37083 might be contributing to this behavior in my environment.

Could you provide more details about the instability you experienced in production? For example:

  • Was it related to UNKNOWN gRPC status codes or issues with the upstream?
  • Did it manifest in specific external processors or all gRPC traffic?

Best regards,
Lin.

adisuissa
adisuissa approved these changes Dec 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@yanavlasov yanavlasov merged commit 66cc217 into envoyproxy:main Dec 20, 2024
@yanavlasov yanavlasov deleted the revert-pr-37083 branch December 20, 2024 15:06
@yanavlasov
Copy link
Copy Markdown
Contributor Author

yanavlasov commented Dec 20, 2024 •
edited
Loading

Hi @yanavlasov,

I noticed that you reverted PR #37083 due to suspected instability in production. I’ve started seeing UNKNOWN gRPC errors and no_healthy_upstream logs in Envoy.

Before this upgrade, even when there were upstream issues, I didn’t encounter UNKNOWN gRPC status codes. I suspect that the stricter enforcement of trailers in PR #37083 might be contributing to this behavior in my environment.

Could you provide more details about the instability you experienced in production? For example:

  • Was it related to UNKNOWN gRPC status codes or issues with the upstream?
  • Did it manifest in specific external processors or all gRPC traffic?

Best regards, Lin.

Your errors will not be caused by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adisuissa adisuissa adisuissa approved these changes

@tyxia tyxia tyxia approved these changes

@mattklein123 mattklein123 Awaiting requested review from mattklein123 mattklein123 is a code owner

Assignees

@stevenzzzz stevenzzzz

@tyxia tyxia

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yanavlasov @LinMoskovitch @adisuissa @tyxia @stevenzzzz