Chukwa
  1. Chukwa
  2. CHUKWA-97

FileTailingAdaptor.tailFile should be refactored and made more flexible

    Details

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

      Description

      FileTailingAdaptor.tailFile() is 150 lines long, and scary. An awful lot lof policy, for things like log file rotation, is hardcoded and shouldn't be.

        Activity

        Ari Rabkin created issue -
        Ari Rabkin made changes -
        Field Original Value New Value
        Component/s data collection [ 12312723 ]
        Ari Rabkin made changes -
        Summary FileTailingAdaptor.tailFile should be refactored FileTailingAdaptor.tailFile should be refactored and made more flexible
        Ari Rabkin made changes -
        Attachment CHUKWA-97.patch [ 12419447 ]
        Hide
        Ari Rabkin added a comment -

        Really, done to get at CHUKWA-361.

        Show
        Ari Rabkin added a comment - Really, done to get at CHUKWA-361 .
        Ari Rabkin made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Ari Rabkin made changes -
        Assignee Ari Rabkin [ asrabkin ]
        Hide
        Ari Rabkin added a comment -

        Will commit within a few days barring objection, and after testing. It's a refactoring, and passes unit tests.

        Show
        Ari Rabkin added a comment - Will commit within a few days barring objection, and after testing. It's a refactoring, and passes unit tests.
        Hide
        Ari Rabkin added a comment -

        Able to tail thousands of files per second, using a small fraction of CPU, negligible memory. Intended to be used in combination with DirTailingAdaptor.

        Show
        Ari Rabkin added a comment - Able to tail thousands of files per second, using a small fraction of CPU, negligible memory. Intended to be used in combination with DirTailingAdaptor.
        Hide
        Jerome Boulon added a comment -

        Ari, is this patch suppose to replace the current fileTailing adaptor? If yes, do you have tested fileRotation, register done but file creation has been delayed. etc with that patch?

        Show
        Jerome Boulon added a comment - Ari, is this patch suppose to replace the current fileTailing adaptor? If yes, do you have tested fileRotation, register done but file creation has been delayed. etc with that patch?
        Hide
        Ari Rabkin added a comment -

        This patch very carefully does not replace the current file tailing adaptor. It does two things.

        1) It makes it easier to write file tailing adaptors that behave differently in the presence of files that have been rotated, deleted, not yet created, etc.
        2) It offers a simple lightweight adaptor to handle the use case where there are thousands of files being watched, and the behavior when files are deleted isn't particularly crucial.

        And yes, unit tests still pass.

        Show
        Ari Rabkin added a comment - This patch very carefully does not replace the current file tailing adaptor. It does two things. 1) It makes it easier to write file tailing adaptors that behave differently in the presence of files that have been rotated, deleted, not yet created, etc. 2) It offers a simple lightweight adaptor to handle the use case where there are thousands of files being watched, and the behavior when files are deleted isn't particularly crucial. And yes, unit tests still pass.
        Hide
        Jerome Boulon added a comment -

        But do we have junit on log rotation ??

        Show
        Jerome Boulon added a comment - But do we have junit on log rotation ??
        Hide
        Ari Rabkin added a comment -

        The refactoring I did shouldn't have changed the code path in FileTailingAdaptor; it just refactored some code into a new base class.

        We do have a test case for rotation. More tests and documentation are still welcome, of course.

        Show
        Ari Rabkin added a comment - The refactoring I did shouldn't have changed the code path in FileTailingAdaptor; it just refactored some code into a new base class. We do have a test case for rotation. More tests and documentation are still welcome, of course.
        Hide
        Ari Rabkin added a comment -

        I want to commit this. Jerome, are you OK with this, or are there revisions you want to see first?

        Show
        Ari Rabkin added a comment - I want to commit this. Jerome, are you OK with this, or are there revisions you want to see first?
        Hide
        Jerome Boulon added a comment -

        +1

        Show
        Jerome Boulon added a comment - +1
        Hide
        Ari Rabkin added a comment -

        I just committed this. Thanks all, for the review.

        Show
        Ari Rabkin added a comment - I just committed this. Thanks all, for the review.
        Ari Rabkin made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Chukwa-trunk #145 (See http://hudson.zones.apache.org/hudson/job/Chukwa-trunk/145/)
        . Refactored FileTailers, added LWFTAdaptor.

        Show
        Hudson added a comment - Integrated in Chukwa-trunk #145 (See http://hudson.zones.apache.org/hudson/job/Chukwa-trunk/145/ ) . Refactored FileTailers, added LWFTAdaptor.

          People

          • Assignee:
            Ari Rabkin
            Reporter:
            Ari Rabkin
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development