buf_file: handle "Too many open files" error to keep buffer and metadata pair #1468
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
repeatedly
force-pushed
the
buffer-with-too-many-open-files
branch
from
387295c
to
27ecd86
Compare
…ata pair This is not incomplete but keep buffer and metadata pair phase as much as possible.
repeatedly
force-pushed
the
buffer-with-too-many-open-files
branch
from
27ecd86
to
332ee73
Compare
|
@tagomoris Could you check this? |
|
@repeatedly Do you have any issues that this pull-request is about? Add it in description if exists. |
tagomoris
requested changes
Feb 16, 2017
Because it can't be restored. We can add retry mechanizm for it but it may cause livelock under too many open and high load environment . |
|
Added comment. |
tagomoris
reviewed
Feb 16, 2017
| # So we ignore such errors for now and log better message instead. | ||
| # "Too many open files" should be fixed by proper buffer configuration and system setting. | ||
| end | ||
| if re |
There was a problem hiding this comment.
It's better to use else clause to simplify code.
begin
file_rename
rescue => re
raise "" # using re
else
raise "" # using e
end| rescue => re | ||
| # See above | ||
| end | ||
| if re |
| @chunk.sync = true | ||
| @chunk.binmode | ||
| rescue => e | ||
| raise BufferOverflowError, "can't create buffer file for #{path}. Stop creating buffer files: error = #{e}" |
There was a problem hiding this comment.
Does this statement expect "too many open files" or exceptions like that?
Please add a comment about expected exception types - it seems not the direct "buffer overflow" error.
| @meta.sync = true | ||
| @meta.binmode | ||
| rescue => e | ||
| @chunk.close |
| @meta.binmode | ||
| rescue => e | ||
| @chunk.close | ||
| File.unlink(@path) |
| rescue => e | ||
| @chunk.close | ||
| File.unlink(@path) | ||
| raise BufferOverflowError, "can't create buffer metadata for #{path}. Stop creating buffer files: error = #{e}" |
tagomoris
approved these changes
Feb 16, 2017
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.
This is not incomplete but keep buffer and metadata pair phase as much as possible.
Use
BufferOverflowErrorincreate_new_chunkfor input plugin retry.TODO: Writing unittest for this case is hard. It will be added to integration test.