Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace assert with warning in error event processing #8058

Merged
merged 2 commits into from
Jul 19, 2021

Conversation

shinrich
Copy link
Member

Fixes an issue introduced in PR #7809.

Replaced the ink_release_assert with a warning so we can gain information about this situation.

@shinrich shinrich added the Crash label Jul 13, 2021
@shinrich shinrich added this to the 10.0.0 milestone Jul 13, 2021
@shinrich shinrich self-assigned this Jul 13, 2021
@shinrich shinrich requested a review from bryancall as a code owner July 13, 2021 16:12
@shinrich
Copy link
Member Author

We have been running this in our production. Vinith looked up the messages we received. 50 over our footprint in a 4 hour period. Most of them are just EPOLLERR (0x8). A least one also had EPOLLHUP (0x18). May be ok to just ignore them. Or to pass them along via a write event as this PR does.

Copy link
Contributor

@jrushford jrushford left a comment

Choose a reason for hiding this comment

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

Looks good to me, I'm going to deploy this change in our 9.1.x testing environment where we've been hitting the assertion that is being replaced here.

Copy link
Contributor

@jrushford jrushford left a comment

Choose a reason for hiding this comment

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

Looks good.

@shinrich shinrich merged commit 6f32b7c into apache:master Jul 19, 2021
zwoop pushed a commit that referenced this pull request Jul 20, 2021
Replace assert with non-release and process as write event in error event processing

(cherry picked from commit 6f32b7c)
@zwoop
Copy link
Contributor

zwoop commented Jul 20, 2021

Cherry-picked to v9.1.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.1.0 Jul 20, 2021
SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request Jul 23, 2021
Replace assert with non-release and process as write event in error event processing
(cherry picked from commit 6f32b7c)

Conflicts:
	iocore/net/UnixNet.cc
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Aug 2, 2021
* asf/9.1.x: (28 commits)
  Updated ChangeLog
  Make the rest of InkAPI allocators Proxy Allocated (apache#8106)
  Added missing milestones and updated slow log report script (apache#8168)
  Cleans up the code bit, including milliseconds consistency (apache#7989)
  Note YAML parser library bug, and work-around, in documentation. (apache#7963)
  Update INSTALL for URLs and version number (apache#8173)
  ESI plugin documentation updates. (apache#7970)
  Add a JSON schema for strategies.yaml (apache#7932)
  ensure hostname_offset is initialized to '0' to indicate null hostname (apache#8162)
  Fixed compile error with Linux AIO unit test (apache#7958)
  Enforce case for well known methods (apache#7886)
  Treat TRACE with body as bad request (apache#7905)
  Close connection after every bad request for HTTP/1.1 (apache#7885)
  use sendmsg and recvmsg (apache#7793)
  Apply log throttling to HTTP/2 session error rate messages (apache#7772)
  limit m_current_range to max value in RangeTransform (apache#4843)
  Fix HPACK eviction iterator manipulation (apache#8004)
  Replace fix assert in error event processing (apache#8058)
  Adds OCSP support for BoringSSL (apache#7298)
  .gitignore rules for gcov generated files (apache#8099)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants