Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-31326

Git Polling isRevExcluded causes OutOfMemoryException

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Critical
    • Resolution: Fixed
    • Component/s: git-client-plugin
    • Labels:
      None
    • Environment:
      git-plugin 2.4.0
      git-client-plugin 1.19.0
      jenkins 1.609.3
    • Similar Issues:

      Description

      In our environment, during git polling (Git Polling Log) the following command causes an OutOfMemoryException attempting to read a 2GB+? String from a ByteArrayOutputStream (~1GB when I copied it from the heap dump to a .txt file).

      This is the offending command:

      /usr/bin/git log --full-history --no-abbrev --format=raw -M -m --raw <previous_build_sha1>..<polling_sha1> # timeout=10 

      The actual error in the build log is:

      ERROR: Connection was broken: java.io.IOException: Unexpected reader termination
      at hudson.remoting.SynchronousCommandTransport$ReaderThread.run(SynchronousCommandTransport.java:76)
      Caused by: java.lang.OutOfMemoryError: Requested array size exceeds VM limit

      This is due to the two commits being 1000's of commits/1000's of files different. When this error occurs the JVM ends up in such a bad state that the ssh-slaves-plugin is unable to relaunch the slaves, and all our machines go offline when this happens on a machine.

      Details: GitSCM.isRevExcluded() calls CliGitAPIImpl.showRevision() which calls the "git log -full-history .." command. Only 3 extensions implement isRevExcluded(): PathRestriction.class, MessageExclusion.class, and UserExclusion.class. These extensions require commit files, commit message, and commit author respectively. Since the vast majority of the size of the ByteArray is from the commit file info, my proposal is to remove the "--raw" if the PathRestriction extension is not being used by the job:

      /usr/bin/git log --full-history --no-abbrev --format=raw -M -m <previous_build_sha1>..<polling_sha1> # timeout=10

        Attachments

          Activity

          Hide
          markewaite Mark Waite added a comment - - edited

          That is an interesting suggestion.

          The Linux kernel I cloned from git://github.com/torvalds/linux.git has about 550 000 commits between be0e5c097fc and c94eee8a3be. When run with the --raw argument, it results in a 9 GB output. When run without the --raw argument, it results in a 500 MB output. I suspect that 500 MB output likely means that the results still can't be processed through a ByteArrayOutputStream, but must be read in smaller chunks, and discarded while reading.

          I'm open to consider a pull request to change that as you suggest, though I fear the ByteArrayOutputStream technique may need to be replaced.

          Show
          markewaite Mark Waite added a comment - - edited That is an interesting suggestion. The Linux kernel I cloned from git://github.com/torvalds/linux.git has about 550 000 commits between be0e5c097fc and c94eee8a3be. When run with the --raw argument, it results in a 9 GB output. When run without the --raw argument, it results in a 500 MB output. I suspect that 500 MB output likely means that the results still can't be processed through a ByteArrayOutputStream, but must be read in smaller chunks, and discarded while reading. I'm open to consider a pull request to change that as you suggest, though I fear the ByteArrayOutputStream technique may need to be replaced.
          Hide
          bjacklyn Brandon Jacklyn added a comment -

          I'm currently working on adding an additional method to GitClient.class called showRevision(ObjectId from, ObjectId to, Boolean excludeCommitFiles). False means "--raw" is included, and this would be the default. This would be called by GitSCM.isRevExcluded() --> showRevision(from, to, true) if the job does not contain a PathRestriction extension, else called normally.

          But yes I think ultimately the more correct solution would be to swap the ByteArrayOutputStream for something else. However, I am not very knowledgeable about OutputStreams. A FileOutputStream might work if it isn't too slow?

          Show
          bjacklyn Brandon Jacklyn added a comment - I'm currently working on adding an additional method to GitClient.class called showRevision(ObjectId from, ObjectId to, Boolean excludeCommitFiles). False means "--raw" is included, and this would be the default. This would be called by GitSCM.isRevExcluded() --> showRevision(from, to, true) if the job does not contain a PathRestriction extension, else called normally. But yes I think ultimately the more correct solution would be to swap the ByteArrayOutputStream for something else. However, I am not very knowledgeable about OutputStreams. A FileOutputStream might work if it isn't too slow?
          Hide
          bjacklyn Brandon Jacklyn added a comment -

          I have been running the code from the following 2 pull requests for about a week now on our production jenkins, and I haven't seen this issue since.

          https://github.com/jenkinsci/git-plugin/pull/362
          https://github.com/jenkinsci/git-client-plugin/pull/190

          Show
          bjacklyn Brandon Jacklyn added a comment - I have been running the code from the following 2 pull requests for about a week now on our production jenkins, and I haven't seen this issue since. https://github.com/jenkinsci/git-plugin/pull/362 https://github.com/jenkinsci/git-client-plugin/pull/190
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Brandon Jacklyn
          Path:
          src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
          src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java
          src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
          src/main/java/org/jenkinsci/plugins/gitclient/RemoteGitImpl.java
          http://jenkins-ci.org/commit/git-client-plugin/5d52c7a076b8e044a35de081dea9522333098729
          Log:
          Add showRevision(from,to,useRawOutput) to support JENKINS-31326

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Brandon Jacklyn Path: src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java src/main/java/org/jenkinsci/plugins/gitclient/RemoteGitImpl.java http://jenkins-ci.org/commit/git-client-plugin/5d52c7a076b8e044a35de081dea9522333098729 Log: Add showRevision(from,to,useRawOutput) to support JENKINS-31326
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Mark Waite
          Path:
          src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
          src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java
          src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
          src/main/java/org/jenkinsci/plugins/gitclient/RemoteGitImpl.java
          src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java
          http://jenkins-ci.org/commit/git-client-plugin/3f70409b4a2bc250721ab952f0b9b880cd98440f
          Log:
          Merge pull request #190 from bjacklyn/user/bjacklyn/git-polling-causes-outofmemoryexception-JENKINS-31326

          Add showRevision(from,to,excludeCommitFiles) to support JENKINS-31326

          Compare: https://github.com/jenkinsci/git-client-plugin/compare/a9f57aa55e42...3f70409b4a2b

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java src/main/java/org/jenkinsci/plugins/gitclient/RemoteGitImpl.java src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java http://jenkins-ci.org/commit/git-client-plugin/3f70409b4a2bc250721ab952f0b9b880cd98440f Log: Merge pull request #190 from bjacklyn/user/bjacklyn/git-polling-causes-outofmemoryexception- JENKINS-31326 Add showRevision(from,to,excludeCommitFiles) to support JENKINS-31326 Compare: https://github.com/jenkinsci/git-client-plugin/compare/a9f57aa55e42...3f70409b4a2b
          Hide
          markewaite Mark Waite added a comment -

          Change included in git client plugin 1.19.6, released 6 Mar 2016. Still needs matching change in the git plugin to use the API added to the git client plugin.

          Show
          markewaite Mark Waite added a comment - Change included in git client plugin 1.19.6, released 6 Mar 2016. Still needs matching change in the git plugin to use the API added to the git client plugin.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Brandon Jacklyn
          Path:
          src/main/java/hudson/plugins/git/GitSCM.java
          http://jenkins-ci.org/commit/git-plugin/54dd4308e635323153c546cbacb9814dbfe59970
          Log:
          Do not read commit log file info during polling unless job uses PathRestriction (JENKINS-31326)

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Brandon Jacklyn Path: src/main/java/hudson/plugins/git/GitSCM.java http://jenkins-ci.org/commit/git-plugin/54dd4308e635323153c546cbacb9814dbfe59970 Log: Do not read commit log file info during polling unless job uses PathRestriction ( JENKINS-31326 )
          Hide
          markewaite Mark Waite added a comment -

          Included in git plugin 2.4.3, released 19 Mar 2016.

          Show
          markewaite Mark Waite added a comment - Included in git plugin 2.4.3, released 19 Mar 2016.

            People

            • Assignee:
              bjacklyn Brandon Jacklyn
              Reporter:
              bjacklyn Brandon Jacklyn
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: