Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.5.0
    • Fix Version/s: 1.6.0, 2.0-M1
    • Component/s: camel-core
    • Labels:
      None

      Description

      Using a memory profiler, we've identified what appears to be a substantial memory leak in FileConsumer in Camel 1.5.0. It appears that the noopMap is constantly having items added to it, but nothing performs a remove on it when the file is consumed. This causes a very large amount of string data to be accumulated in the heap. In our application, this was a leak of several hundred megabytes and is a showstopper. Considering the apparent severity of this issue, it would really be nice if a fix could be incorporated into a 1.5.1 version.

      http://www.nabble.com/Memory-leak-in-FileConsumer-in-Camel-1.5.0-td20794405s22882.html#a20794405

        Activity

        Hide
        davsclaus Claus Ibsen added a comment -

        This is fixed in 2.0 as we have removed all this troublesome code.

        For 1.5.1 I suggest to replace the noopMap with the LRUCahce Map so it keeps up till 1000 files and nothing more.

        Show
        davsclaus Claus Ibsen added a comment - This is fixed in 2.0 as we have removed all this troublesome code. For 1.5.1 I suggest to replace the noopMap with the LRUCahce Map so it keeps up till 1000 files and nothing more.
        Hide
        davsclaus Claus Ibsen added a comment -

        These maps is part of some code logic to determine if the file has been changed using timestamp and filesize checks. All this code has been @deprecated and removed in 2.0.
        It leads to unexpected behavior and is hard to test. And shouldn't generally be used.

        If you use file connectivity then you should either delete or move files after they are processed and not keep then around.
        As a fix for this in 1.5.1 I have added the LRUCache so the maps will contain at most 1000 elements.

        Show
        davsclaus Claus Ibsen added a comment - These maps is part of some code logic to determine if the file has been changed using timestamp and filesize checks. All this code has been @deprecated and removed in 2.0. It leads to unexpected behavior and is hard to test. And shouldn't generally be used. If you use file connectivity then you should either delete or move files after they are processed and not keep then around. As a fix for this in 1.5.1 I have added the LRUCache so the maps will contain at most 1000 elements.
        Hide
        davsclaus Claus Ibsen added a comment -

        D:\project\camel-1x\camel-core>svn commit --message "CAMEL-1138: Quick fix for file consumer not freeing memory"
        Sending camel-core\src\main\java\org\apache\camel\component\file\FileConsumer.java
        Transmitting file data .
        Committed revision 722808.

        Show
        davsclaus Claus Ibsen added a comment - D:\project\camel-1x\camel-core>svn commit --message " CAMEL-1138 : Quick fix for file consumer not freeing memory" Sending camel-core\src\main\java\org\apache\camel\component\file\FileConsumer.java Transmitting file data . Committed revision 722808.
        Hide
        davsclaus Claus Ibsen added a comment -

        Hadrian is the current quick fix okay?

        Show
        davsclaus Claus Ibsen added a comment - Hadrian is the current quick fix okay?
        Hide
        hadrian Hadrian Zbarcea added a comment -

        I think so. It actually solves the leak problem.

        The side effect is that if a file is not moved/renamed after 1000 other entries/files it will disappear from the lru cache and look new again and get reprocessed, which may also lead to an infinite loop (that would make another one of those 1000+ files new/out of cache, and so on). However I don't think this scenario is a valid one as it has other flaws, such as the fact that the state is lost after a camel restart, so files would get processed again in that case too (a comment in the code mentions that, actually).

        I would add a warn in the wiki that not renaming/moving files is a dangerous scenario and should not be used.

        Show
        hadrian Hadrian Zbarcea added a comment - I think so. It actually solves the leak problem. The side effect is that if a file is not moved/renamed after 1000 other entries/files it will disappear from the lru cache and look new again and get reprocessed, which may also lead to an infinite loop (that would make another one of those 1000+ files new/out of cache, and so on). However I don't think this scenario is a valid one as it has other flaws, such as the fact that the state is lost after a camel restart, so files would get processed again in that case too (a comment in the code mentions that, actually). I would add a warn in the wiki that not renaming/moving files is a dangerous scenario and should not be used.
        Hide
        hadrian Hadrian Zbarcea added a comment -

        I think we can close this issue.

        Show
        hadrian Hadrian Zbarcea added a comment - I think we can close this issue.

          People

          • Assignee:
            davsclaus Claus Ibsen
            Reporter:
            hadrian Hadrian Zbarcea
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development