Flume
  1. Flume
  2. FLUME-1350

HDFS file handle not closed properly when date bucketing

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: v1.1.0, v1.2.0
    • Fix Version/s: None
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      With configuration:

      agent.sinks.hdfs-cafe-access.type = hdfs
      agent.sinks.hdfs-cafe-access.hdfs.path = hdfs://nga/nga/apache/access/%y-%m-%d/
      agent.sinks.hdfs-cafe-access.hdfs.fileType = DataStream
      agent.sinks.hdfs-cafe-access.hdfs.filePrefix = cafe_access
      agent.sinks.hdfs-cafe-access.hdfs.rollInterval = 21600
      agent.sinks.hdfs-cafe-access.hdfs.rollSize = 10485760
      agent.sinks.hdfs-cafe-access.hdfs.rollCount = 0
      agent.sinks.hdfs-cafe-access.hdfs.txnEventMax = 1000
      agent.sinks.hdfs-cafe-access.hdfs.batchSize = 1000
      #agent.sinks.hdfs-cafe-access.hdfs.codeC = snappy
      agent.sinks.hdfs-cafe-access.hdfs.hdfs.maxOpenFiles = 5000
      agent.sinks.hdfs-cafe-access.channel = memo-1

      When new directory is created previous file handle remains opened. rollInterval setting is used only with files in current date bucket.

        Issue Links

          Activity

          Hide
          Yongcheng Li added a comment -

          This is my patch for FLUME-1350. I've tested it on my Hadoop system and it worked.

          Show
          Yongcheng Li added a comment - This is my patch for FLUME-1350 . I've tested it on my Hadoop system and it worked.
          Hide
          Yongcheng Li added a comment -

          I've been testing my patch and haven't seen any problem yet. Anyone else done any testing? How to move forward to have this patch reviewed and submitted?

          Show
          Yongcheng Li added a comment - I've been testing my patch and haven't seen any problem yet. Anyone else done any testing? How to move forward to have this patch reviewed and submitted?
          Hide
          Brock Noland added a comment -

          Hi,

          please submit the patch on reviews.apache.org (example https://reviews.apache.org/r/6411/), add the review as a "link" to the JIRA (More Actions -> Link), and then click "Submit Patch" so the status is "Patch Available".

          Show
          Brock Noland added a comment - Hi, please submit the patch on reviews.apache.org (example https://reviews.apache.org/r/6411/ ), add the review as a "link" to the JIRA (More Actions -> Link), and then click "Submit Patch" so the status is "Patch Available".
          Hide
          Brock Noland added a comment -

          Also, looking at the patch the formatting is off? Make sure you use 2 spaces for a tab. Additionally the Override method appears to have been commented out:

          + */ @Override

          Show
          Brock Noland added a comment - Also, looking at the patch the formatting is off? Make sure you use 2 spaces for a tab. Additionally the Override method appears to have been commented out: + */ @Override
          Hide
          Brock Noland added a comment -

          Can you attach the patch to the JIRA?

          Show
          Brock Noland added a comment - Can you attach the patch to the JIRA?
          Hide
          Yongcheng Li added a comment -

          Patch file for HDFSEventSink.java.

          Show
          Yongcheng Li added a comment - Patch file for HDFSEventSink.java.
          Hide
          Mike Percy added a comment -

          Moving state back to Unresolved based on feedback provided on review board.

          Show
          Mike Percy added a comment - Moving state back to Unresolved based on feedback provided on review board.
          Hide
          Mike Percy added a comment -

          This is not a bug from what I have seen. You have to specify a rollInterval if you want time-based rolling.

          Please provide more info if you want to pursue this otherwise let's mark this as Not a Bug.

          Show
          Mike Percy added a comment - This is not a bug from what I have seen. You have to specify a rollInterval if you want time-based rolling. Please provide more info if you want to pursue this otherwise let's mark this as Not a Bug.
          Hide
          Yongcheng Li added a comment - - edited

          This is a bug and has been reported frequently in the mailing list. Actually, it is very easy to reproduce which you should have done.

          Show
          Yongcheng Li added a comment - - edited This is a bug and has been reported frequently in the mailing list. Actually, it is very easy to reproduce which you should have done.
          Hide
          Mike Percy added a comment -

          Flume is not designed to close the file until a certain event or condition triggers the close. Please also remember that Flume's file path timestamps are event-oriented.
          The following operations will close the file:

          1. rollInterval != 0: will close the file after N seconds have passed since the first event was written to it
          2. rollCount != 0: will close the file after N events have been written to it
          3. rollSize != 0: will close the file after N body bytes have been written to it
          4. The number of open files increases beyond maxOpenFiles
          5. Flume shuts down

          Have you met any of these conditions in the files that are remaining open?

          Based on rollInterval = 21600, you will not close the file automatically until 6 hours has passed after the first event is written to the file.

          Show
          Mike Percy added a comment - Flume is not designed to close the file until a certain event or condition triggers the close. Please also remember that Flume's file path timestamps are event-oriented. The following operations will close the file: 1. rollInterval != 0: will close the file after N seconds have passed since the first event was written to it 2. rollCount != 0: will close the file after N events have been written to it 3. rollSize != 0: will close the file after N body bytes have been written to it 4. The number of open files increases beyond maxOpenFiles 5. Flume shuts down Have you met any of these conditions in the files that are remaining open? Based on rollInterval = 21600, you will not close the file automatically until 6 hours has passed after the first event is written to the file.
          Hide
          Yongcheng Li added a comment -

          Can you explain what does date bucketing mean?

          What is the expected behavior if my configuration is:

          agent.sinks.hdfs-cafe-access.hdfs.path = hdfs://nga/nga/apache/access/%y-%m-%d/

          Show
          Yongcheng Li added a comment - Can you explain what does date bucketing mean? What is the expected behavior if my configuration is: agent.sinks.hdfs-cafe-access.hdfs.path = hdfs://nga/nga/apache/access/%y-%m-%d/
          Hide
          Mike Percy added a comment -

          That path means that any Event that goes to the HDFS sink must have a header called "timestamp" which is a stringified Long value, typical Java timestamp in milliseconds. The year-month-day will be generated from that timestamp, and the event will be stored in a file under that directory.

          If there is already an open file in that directory, the event will be appended to that file. If there is no open file in that directory, a new file will be created.

          The only rules for closing a file are listed above, because when events are collected from many hosts, there may be old events coming through at the same time as new events, and we would not want to create too many small files. So, the time to allow a file to remain open is configurable before automatically closing it using rollInterval.

          Show
          Mike Percy added a comment - That path means that any Event that goes to the HDFS sink must have a header called "timestamp" which is a stringified Long value, typical Java timestamp in milliseconds. The year-month-day will be generated from that timestamp, and the event will be stored in a file under that directory. If there is already an open file in that directory, the event will be appended to that file. If there is no open file in that directory, a new file will be created. The only rules for closing a file are listed above, because when events are collected from many hosts, there may be old events coming through at the same time as new events, and we would not want to create too many small files. So, the time to allow a file to remain open is configurable before automatically closing it using rollInterval.
          Hide
          Yongcheng Li added a comment -

          Is it required to use rollinterval when using data bucketing? In real world, you don't want to use rollinterval to generate many small files. Instead, it's common to use rollSize combined with data bucketing.

          When using rollSize and data backeting, since no more data will be written into the old file, the file may never be closed until the system is down or there are too many open files, which is not the desired/expected behavior and that's why so many people complained about this bug.

          Show
          Yongcheng Li added a comment - Is it required to use rollinterval when using data bucketing? In real world, you don't want to use rollinterval to generate many small files. Instead, it's common to use rollSize combined with data bucketing. When using rollSize and data backeting, since no more data will be written into the old file, the file may never be closed until the system is down or there are too many open files, which is not the desired/expected behavior and that's why so many people complained about this bug.
          Hide
          Mike Percy added a comment -

          In production deployments that I have seen, people often use a rollInterval of 5 or 10 minutes to generate the rolled log files. Depending on how many concurrent log files you are writing, this is usually fine for long-term data storage if you are worried about Namenode memory usage. By the way, rollInterval will not generate empty files over periods of inactivity, if you are worried about that.

          If you only want one file open at a time then you can set maxOpenFiles = 1 and get that behavior. However, in a real-world scenario, where you have concurrency and multiple tiers, likely resulting in out-of-order delivery, it is highly unlikely you really want only one file open at a time, since that will result in thrashing.

          Does that help?

          Show
          Mike Percy added a comment - In production deployments that I have seen, people often use a rollInterval of 5 or 10 minutes to generate the rolled log files. Depending on how many concurrent log files you are writing, this is usually fine for long-term data storage if you are worried about Namenode memory usage. By the way, rollInterval will not generate empty files over periods of inactivity, if you are worried about that. If you only want one file open at a time then you can set maxOpenFiles = 1 and get that behavior. However, in a real-world scenario, where you have concurrency and multiple tiers, likely resulting in out-of-order delivery, it is highly unlikely you really want only one file open at a time, since that will result in thrashing. Does that help?
          Hide
          Yongcheng Li added a comment -

          Let me repeat it again, is it required to define rollinterval when using data bucketing? If not, then your argument is just wrong.

          Show
          Yongcheng Li added a comment - Let me repeat it again, is it required to define rollinterval when using data bucketing? If not, then your argument is just wrong.
          Hide
          Mike Percy added a comment -

          The way it's designed today, I'd recommend setting either rollInterval or maxOpenFiles. Please see my previous comment.

          Show
          Mike Percy added a comment - The way it's designed today, I'd recommend setting either rollInterval or maxOpenFiles. Please see my previous comment.
          Hide
          Juhani Connolly added a comment -

          I don't see this patch as particularly harmful, and it does address a minor niggle we also have.

          We roll our files on the hour(so with an hdfs path something like %y%m%d/%h) and have the roll time set to a bit over an hour, which does close the old handles. But it feels like a kludge, and I think that modifying the design to close old handles as the destination bucket changes is not unreasonable. I think offering multiple rolling strategies, but then forcing people to add an unrelated rollInterval to make sure they eventually get closed is not exactly intuitive.

          Can you think of a case where the old writer getting closed would be harmful?

          With that being said, once a path stops receiving writes, I could see the rollInterval still being necessary to close the final file.

          Show
          Juhani Connolly added a comment - I don't see this patch as particularly harmful, and it does address a minor niggle we also have. We roll our files on the hour(so with an hdfs path something like %y%m%d/%h) and have the roll time set to a bit over an hour, which does close the old handles. But it feels like a kludge, and I think that modifying the design to close old handles as the destination bucket changes is not unreasonable. I think offering multiple rolling strategies, but then forcing people to add an unrelated rollInterval to make sure they eventually get closed is not exactly intuitive. Can you think of a case where the old writer getting closed would be harmful? With that being said, once a path stops receiving writes, I could see the rollInterval still being necessary to close the final file.
          Hide
          Mike Percy added a comment - - edited

          Hey Juhani, yes as I mentioned before, when events are collected from many hosts, there may be old events coming through at the same time as new events, and we would not want to create a situation where we are "thrashing" (opening / closing many files).

          As mentioned, if you want that behavior (only one file open at once), you can set maxOpenFiles = 1.

          Show
          Mike Percy added a comment - - edited Hey Juhani, yes as I mentioned before, when events are collected from many hosts, there may be old events coming through at the same time as new events, and we would not want to create a situation where we are "thrashing" (opening / closing many files). As mentioned, if you want that behavior (only one file open at once), you can set maxOpenFiles = 1.
          Hide
          Yongcheng Li added a comment -

          When data bucketing, if you want to accommodate old (out of order) events, you could add a delay closing the old files. But it just does not make sense to a user that when using data bucketing, you have to set rollInterval or set maxOpenFiles = 1.

          Show
          Yongcheng Li added a comment - When data bucketing, if you want to accommodate old (out of order) events, you could add a delay closing the old files. But it just does not make sense to a user that when using data bucketing, you have to set rollInterval or set maxOpenFiles = 1.
          Hide
          Mike Percy added a comment -

          Hey Yongcheng, as long as it's configurable and doesn't negatively impact existing behavior then that sounds reasonable. If you want to submit a patch for that I'd be happy to review it.

          Show
          Mike Percy added a comment - Hey Yongcheng, as long as it's configurable and doesn't negatively impact existing behavior then that sounds reasonable. If you want to submit a patch for that I'd be happy to review it.
          Hide
          Juhani Connolly added a comment -

          Hi Mike, your point is true... if one host gets backed up and ends up sending older logs, then in the case of files named by date there could be some awkwardness. Most solutions do appear to have some outstanding issues so I hope Yongcheng can add the appropriate configuration and documentation of the behavior of his settings so people can understand the consequences. Giving extra options when the current ones are not ideal should be a good thing

          Show
          Juhani Connolly added a comment - Hi Mike, your point is true... if one host gets backed up and ends up sending older logs, then in the case of files named by date there could be some awkwardness. Most solutions do appear to have some outstanding issues so I hope Yongcheng can add the appropriate configuration and documentation of the behavior of his settings so people can understand the consequences. Giving extra options when the current ones are not ideal should be a good thing
          Hide
          Juhani Connolly added a comment -

          I had a more detailed look at the source and I don't really think as it is it will solve a problem without introducing a new one(the thrashing that Mike referred to).

          An alternative solution I would suggest is tracking last writes to each open file, and having a watcher thread close them after a configured timeout period where the file has received no writes. This would solve every case of unclused idle files I can think of, and if a file becomes active again due to a temporarily out of commission source reactivating, it would not result in thrashing(the file would be reopened, and then closed again soon after all the backlog has been handled)

          Yongcheng/Mike: What do you think? If no-one else wants to do it, I can put it together.

          Show
          Juhani Connolly added a comment - I had a more detailed look at the source and I don't really think as it is it will solve a problem without introducing a new one(the thrashing that Mike referred to). An alternative solution I would suggest is tracking last writes to each open file, and having a watcher thread close them after a configured timeout period where the file has received no writes. This would solve every case of unclused idle files I can think of, and if a file becomes active again due to a temporarily out of commission source reactivating, it would not result in thrashing(the file would be reopened, and then closed again soon after all the backlog has been handled) Yongcheng/Mike: What do you think? If no-one else wants to do it, I can put it together.
          Hide
          Yongcheng Li added a comment -

          Juhani, that sounds really a nice solution to this problem. Thanks!

          Show
          Yongcheng Li added a comment - Juhani, that sounds really a nice solution to this problem. Thanks!
          Hide
          Mike Percy added a comment -

          Hi Juhani, something like a close-on-idle timeout makes sense. I'd be happy to review it if you want to work on it.

          Show
          Mike Percy added a comment - Hi Juhani, something like a close-on-idle timeout makes sense. I'd be happy to review it if you want to work on it.
          Hide
          Juhani Connolly added a comment -

          I decided to make this a separate issue in case we still want this one for alternative discussion. I made a pass at the idle timeout closing in FLUME-1660 review is https://reviews.apache.org/r/7659/

          Show
          Juhani Connolly added a comment - I decided to make this a separate issue in case we still want this one for alternative discussion. I made a pass at the idle timeout closing in FLUME-1660 review is https://reviews.apache.org/r/7659/
          Hide
          Juhani Connolly added a comment -

          Hi Mike, any chance you could review that patch before other changes pile up?

          Show
          Juhani Connolly added a comment - Hi Mike, any chance you could review that patch before other changes pile up?
          Hide
          Mike Percy added a comment -

          Hi Juhani, sorry for the delay, I was out of town. I just posted to the review board.

          Show
          Mike Percy added a comment - Hi Juhani, sorry for the delay, I was out of town. I just posted to the review board.

            People

            • Assignee:
              Unassigned
              Reporter:
              Robert Mroczkowski
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:

                Development