Chukwa
  1. Chukwa
  2. CHUKWA-675

Bad sleeping condition in FileTailer.java

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.6.0
    • Fix Version/s: 0.6.0
    • Component/s: Data Collection
    • Labels:
      None

      Description

      Current "if condition" in the run() method is wrong.
      As a consequence, a big file to tail will take a lot of time.

      See: http://mail-archives.apache.org/mod_mbox/incubator-chukwa-dev/201212.mbox/%3CCAOTz1wD-abkJwh5GBnUi3o6xj2kiqYf7HvGDg80mdEk4h6y6+w@mail.gmail.com%3E

      1. chukwa-675.patch
        3 kB
        Sourygna Luangsay

        Activity

        Hide
        Ari Rabkin added a comment -

        I'm a little hesitant with this. I believe the code is the way it is for rate limiting. Is there a production need for this change?

        Show
        Ari Rabkin added a comment - I'm a little hesitant with this. I believe the code is the way it is for rate limiting. Is there a production need for this change?
        Hide
        Ari Rabkin added a comment -

        Also, has this been tried in production?

        Show
        Ari Rabkin added a comment - Also, has this been tried in production?
        Hide
        Sourygna Luangsay added a comment -

        I haven't tried it in production yet. Nonetheless, I have included this change in our benchmarks configuration:

        • without the change, the agent needs more or less 30 minutes to send one 490MB file with the file tailer adaptor.
        • with the change, it only needs 45 seconds

        I don't really understand what you mean by "rate limiting". The code already offers some protection
        to ensure that one (big) file to tail may not prevent other smaller files to be tailed (see chukwaAgent.fileTailingAdaptor.maxReadSize).

        What is more, I think my change is correct if you consider the comment at the beginning of the class:

        • For now, it tries each file in succession. If it gets through every file
        • within two seconds, and no more data remain, it will sleep.
        • If there was still data available in any file, the adaptor will loop again.
        Show
        Sourygna Luangsay added a comment - I haven't tried it in production yet. Nonetheless, I have included this change in our benchmarks configuration: without the change, the agent needs more or less 30 minutes to send one 490MB file with the file tailer adaptor. with the change, it only needs 45 seconds I don't really understand what you mean by "rate limiting". The code already offers some protection to ensure that one (big) file to tail may not prevent other smaller files to be tailed (see chukwaAgent.fileTailingAdaptor.maxReadSize). What is more, I think my change is correct if you consider the comment at the beginning of the class: For now, it tries each file in succession. If it gets through every file within two seconds, and no more data remain, it will sleep. If there was still data available in any file, the adaptor will loop again.
        Hide
        Sourygna Luangsay added a comment -

        If we want to limit network bandwidth consumption, I think we can use httpConnector.maxPostSize, no?

        Show
        Sourygna Luangsay added a comment - If we want to limit network bandwidth consumption, I think we can use httpConnector.maxPostSize, no?
        Hide
        Eric Yang added a comment -

        If there is data, adaptor should be eager to send data. If there is no new data, then agent pause 2 seconds until next check. I think this is a good change. We had some busy loop issues in the past, and it was because there is no pause even after there is no new data to stream. I think the new patch is the proper behavior.

        Show
        Eric Yang added a comment - If there is data, adaptor should be eager to send data. If there is no new data, then agent pause 2 seconds until next check. I think this is a good change. We had some busy loop issues in the past, and it was because there is no pause even after there is no new data to stream. I think the new patch is the proper behavior.
        Hide
        Ari Rabkin added a comment -

        OK. Fair enough. Let's check it in and see what happens.

        Show
        Ari Rabkin added a comment - OK. Fair enough. Let's check it in and see what happens.
        Hide
        Ari Rabkin added a comment -

        I just committed this. Thanks!

        Show
        Ari Rabkin added a comment - I just committed this. Thanks!
        Hide
        Hudson added a comment -

        Integrated in Chukwa-trunk #461 (See https://builds.apache.org/job/Chukwa-trunk/461/)
        CHUKWA-675. Bad sleeping condition in FileTailer.java. Contributed by Sourygna Luangsay. (Revision 1423254)

        Result = SUCCESS
        asrabkin : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1423254
        Files :

        • /incubator/chukwa/trunk/CHANGES.txt
        • /incubator/chukwa/trunk/src/main/java/org/apache/hadoop/chukwa/datacollection/adaptor/filetailer/FileTailer.java
        Show
        Hudson added a comment - Integrated in Chukwa-trunk #461 (See https://builds.apache.org/job/Chukwa-trunk/461/ ) CHUKWA-675 . Bad sleeping condition in FileTailer.java. Contributed by Sourygna Luangsay. (Revision 1423254) Result = SUCCESS asrabkin : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1423254 Files : /incubator/chukwa/trunk/CHANGES.txt /incubator/chukwa/trunk/src/main/java/org/apache/hadoop/chukwa/datacollection/adaptor/filetailer/FileTailer.java

          People

          • Assignee:
            Sourygna Luangsay
            Reporter:
            Sourygna Luangsay
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development