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

FLUME-3217 . Creates empty files when quota #225

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from

Conversation

ffernandez92
Copy link

As it can be seen in FLUME-3217, Flume creates empty files when HDFS quota has been reached.

This is a first approach to the solution. It works although a better approach it could be implemented.
The idea is to capture the quota Exception in order to delete this annoying empty files generated by flume.

As it can be seen in FLUME-3217, Flume creates empty files when HDFS quota has been reached.

This is a first approach to the solution, it works although a better approach it could be implemented. The idea is to capture the quota Exception in order to delete this annoying empty files generated by flume.
Copy link
Contributor

@szaboferee szaboferee left a comment

Choose a reason for hiding this comment

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

@ffernandez92 thanks for the contribution

are you sure that the DSQuotaExceededException can only happen with empty files and it is safe to delete them? You can configure flume to write files with the size of gigabytes and the quota may exceed in the middle of that.

@asfgit
Copy link

asfgit commented Aug 17, 2018

Can one of the admins verify this patch?

@ffernandez92
Copy link
Author

Hi @szaboferee I have reproduced the environment and you are right.
I could happen that if the quota is reached in the middle of a “transaction”, the all file dumps.

It is necessary to check if the file weight is zero before doing any delete operation. Moreover it will be necessary to implement some sort of “stopper” in the “append” function of the BucketWriter class in order to achieve the expected behavior.

I will try to do the modifications asap. Thank you and sorry for the stupid mistake.

Avoid the deletion of empty files when quota reached as it could delete a file that does not have to be deleted.
Adding the checkQuota function:
-	When the quota is reached avoids creating new files as interrupts the open flow
-	If the quota is reached just in the middle of a transaction, the file is closed with the data that could be included and then any other file will be created.
@szaboferee
Copy link
Contributor

@ffernandez92 can you add a test with a minicluster? It would make the review much easier

Check FS type before doing any action.
@ffernandez92
Copy link
Author

Hi @szaboferee sure, I will add a test with minicluster. Now I am uploading a new version as I am having some trouble with test part in local.

There was a mistake when the replication factor was greater than 1. I expect to commit a test shortly
Copy link
Contributor

@szaboferee szaboferee left a comment

Choose a reason for hiding this comment

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

@ffernandez92 Thanks, it looks promising, I have one question between the lines.

config.setBoolean("fs.automatic.close", false);
ContentSummary cSumm;
try {
cSumm = fsCheckQuota(quotaPath.getFileSystem(config),quotaPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering if checking the quota for every single write would cause any performance degradation.
Would it make sense to not compute the size of the event but a configurable threshold?
Like flume would stop writing if there is only 100 MB left from the quota. This way we could avoid race conditions for the last event sized free space.

@majorendre
Copy link
Contributor

@ffernandez92 thanks for the contribution.

I guess this problem can be quite painful.
For performance reasons I would go another way to fix this problem. I would add a check to the close() function. It would check after successful close if the file length is 0. That is simple to implement and would happen only after a close. If the close was not called from a catch block, we could omit this for further speed.
I guess, still there could be a problem when the quota is exceeded, Flume can get into a sort of infinite loop as it tries to write the same event to HDFS again and again. Some sort off backoff should be implemented to reduce load. Needs to be checked if it is true.

Let me know please, what you think.

@ffernandez92
Copy link
Author

Hi!

Sorry for the late answer I’ve been a little bit under a rush lately. I think you are right @szaboferee about the performance degradation.
I think that the solution you are proposing @majorendre it was also my first approach. But it was also problematic because of what you were saying about the infinite loop when quota was reached in the middle of a write.

I will try to reformulate the solution trying both approaches to see which one would be better.

waidr pushed a commit to waidr/flume that referenced this pull request Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants