feature: use ringBuffer to make log not blocking and make it configurable #2428
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
Codecov Report
@@ Coverage Diff @@
## master #2428 +/- ##
==========================================
- Coverage 69.2% 69.11% -0.09%
==========================================
Files 277 278 +1
Lines 18330 18387 +57
==========================================
+ Hits 12685 12709 +24
- Misses 4206 4242 +36
+ Partials 1439 1436 -3
|
fuweid
reviewed
Nov 6, 2018
daemon/mgr/container_logger.go
Outdated
| } | ||
|
|
||
| if info.LogConfig["mode"] == LogNonBlocking { | ||
| return bufferlog.NewBufferLog(ld, info) |
There was a problem hiding this comment.
could we postpone to start the goroutine after startLogging?
HusterWan
reviewed
Nov 6, 2018
HusterWan
reviewed
Nov 6, 2018
HusterWan
reviewed
Nov 6, 2018
HusterWan
reviewed
Nov 6, 2018
ZYecho
reviewed
Nov 7, 2018
starnop
force-pushed
the
not-blocking
branch
3 times, most recently
from
d7e2c97 to
c6e6f14
Compare
starnop
force-pushed
the
not-blocking
branch
5 times, most recently
from
70b8fe0 to
5eaadc6
Compare
starnop
changed the title
|
@HusterWan @fuweid All mentioned above has been updated. PTAL. THX. |
fuweid
suggested changes
Nov 20, 2018
starnop
force-pushed
the
not-blocking
branch
2 times, most recently
from
8d3b1b7 to
852dc7f
Compare
|
Basically, LGTM. But, @starnop could you help to add benchmark cases for the logbuffer? |
fuweid
reviewed
Nov 21, 2018
starnop
force-pushed
the
not-blocking
branch
from
852dc7f to
75826f5
Compare
Maybe add benchmark cases in the next PR? |
|
oops. @starnop could you help to take a look for this? |
Sure. |
starnop
force-pushed
the
not-blocking
branch
from
75826f5 to
d560486
Compare
Has been fixed. |
starnop
force-pushed
the
not-blocking
branch
from
d560486 to
7ad6bbe
Compare
fuweid
reviewed
Nov 22, 2018
starnop
force-pushed
the
not-blocking
branch
2 times, most recently
from
4cdf322 to
32d02ed
Compare
fuweid
reviewed
Nov 22, 2018
Signed-off-by: Starnop <starnop@163.com>
starnop
force-pushed
the
not-blocking
branch
from
32d02ed to
b4d3081
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Starnop starnop@163.com
Ⅰ. Describe what this PR did
As the title describes.
Ⅱ. Does this pull request fix one issue?
None.
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
Add it Later.
Ⅳ. Describe how to verify it
Add it Later.
Ⅴ. Special notes for reviews