This repository has been archived by the owner on Aug 19, 2022. It is now read-only.
include current limits in debug messages #42
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@marten-seemann : can you please post an example error message before vs now? |
vyzo
reviewed
Jun 6, 2022
marten-seemann
force-pushed
the
rework-errors
branch
2 times, most recently
from
59c71ed to
bf5dd82
Compare
|
@vyzo Re-added the stat. @BigLep after: |
|
Actually, we shouldn't include the |
marten-seemann
force-pushed
the
rework-errors
branch
from
bf5dd82 to
6bdbf3a
Compare
|
Included the stat in a non-racy way (by collecting it at the same time we generate the error, while holding the mutex). This should be ready for review now. |
vyzo
reviewed
Jun 19, 2022
marten-seemann
force-pushed
the
rework-errors
branch
from
3d5d6c2 to
1910cf4
Compare
vyzo
reviewed
Jun 19, 2022
scope.go
Outdated
| @@ -239,7 +329,14 @@ func (s *resourceScope) ReserveMemory(size int, prio uint8) error { | |||
| } | |||
|
|
|||
| if err := s.rc.reserveMemory(int64(size), prio); err != nil { | |||
| log.Debugw("blocked memory reservation", "scope", s.name, "size", size, "priority", prio, "stat", s.rc.stat(), "error", err) | |||
| logValues := make([]interface{}, 0, 7) | |||
There was a problem hiding this comment.
can we abstract this in a function? It adds noise in the logic.
scope.go
Outdated
| var stat network.ScopeStat | ||
| stat, err = e.ReserveMemoryForChild(int64(size), prio) | ||
| if err != nil { | ||
| logValues := make([]interface{}, 0, 7) |
scope.go
Outdated
| @@ -344,7 +450,14 @@ func (s *resourceScope) AddStream(dir network.Direction) error { | |||
| } | |||
|
|
|||
| if err := s.rc.addStream(dir); err != nil { | |||
| log.Debugw("blocked stream", "scope", s.name, "direction", dir, "stat", s.rc.stat(), "error", err) | |||
| logValues := make([]interface{}, 0, 7) | |||
scope.go
Outdated
| var stat network.ScopeStat | ||
| stat, err = e.AddStreamForChild(dir) | ||
| if err != nil { | ||
| logValues := make([]interface{}, 0, 8) |
scope.go
Outdated
| @@ -444,7 +566,14 @@ func (s *resourceScope) AddConn(dir network.Direction, usefd bool) error { | |||
| } | |||
|
|
|||
| if err := s.rc.addConn(dir, usefd); err != nil { | |||
| log.Debugw("blocked connection", "scope", s.name, "direction", dir, "usefd", usefd, "stat", s.rc.stat(), "error", err) | |||
| logValues := make([]interface{}, 0, 8) | |||
scope.go
Outdated
| var stat network.ScopeStat | ||
| stat, err = e.AddConnForChild(dir, usefd) | ||
| if err != nil { | ||
| logValues := make([]interface{}, 0, 8) |
|
@vyzo Moved to separate functions. The reason I chose to have separate functions for stream, conn and memory errors is that I want to avoid allocations: The |
vyzo
reviewed
Jun 19, 2022
scope.go
Outdated
| @@ -18,6 +19,80 @@ type resources struct { | |||
| memory int64 | |||
| } | |||
|
|
|||
| type errStreamOrConnLimitExceeded struct { | |||
There was a problem hiding this comment.
can we put all that stuff at the bottom of the file?
New readers shouldnt be welcomed with an incomprehensible blob of logging arcana.
There was a problem hiding this comment.
Moved it to a separate file.
41 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #26.
Potentially controversial: also removes the
statfrom the debug output, to focus on the thing that is actually getting blocked. We can add it back if preferred.