gitblit-org / gitblit

pure java git solution
http://gitblit.com
Apache License 2.0
2.28k stars 670 forks source link

Tomszt/hang on commit #1387

Open TomaszSzt opened 2 years ago

TomaszSzt commented 2 years ago

SOLVED PROBLEM

Commit page consumes to much time and memory when being subjected to huge, binary mostly commit. In effect server freezes and reset is necessary.

Issue reported: https://github.com/gitblit/gitblit/issues/1385

DESCRIPTION:

This is a pull request which basically does following:

  1. Prevents commit page from chewing up too many files. The number of files is actually configurable through "defaults.properties" web.maxCommitPaths option.
  2. Extends user access to logging configuration by providing the ability to append options seen in $basepath/log4j.properties to options seen in internally jar contained log4j.properties. This is explained in "defaults.properties" web.debugMode option description and in example log4-debug-example.properties file. 2.1 Adds some logging instructions across the application at TRACE level. 2.2. Adds early messages displaying logging configuration on System.out if any --dailyLogFile or web.debugMode or log4.properties was used to override default settings.
  3. Prevents server from sending "Internal Error" if some text resources are missing and using default dumb text instead. This is basically a robustness against human mistakes during development and testing. This is easy to loose some text by mistake. This is especially true with GitBlit context-less keys like: gb.mirrorWarning which do encourage wide use of same text in many places.

Each commit carefully describes steps taken.

TESTING

Only manual tests were made on the problematic repository. Only GitBlit GO was tested.

No new automatic tests are included since:

Old tests are FAILING but they do fail on "master" too. I do not think they are kept up to date or something in my setup is screwed up. I do not feel competent to fix it.

MANUAL TEST CONDITIONS PART 1.Commit pages.

FAULT: Following manual test conditions should apply: 1.Build server 2.Ensure that web.maxCommitPaths=-1 3.Check that Your JVM has 256MB max heap size (default for JDK8) 4.Start server. 5.Create repository with an initial commit. 6.Clone it. 7.Create a huge commit of at least 16k files and at least 1GB in raw size. 8.Push it. 6.Enter it through web interface. Click the commit page. Observe that page takes about 30 seconds to load or more, depending on machine (mine server: 2 minutes) 7.Do it few times, quickly, not waiting for page to load. 8.Try to display any other page, ie. repository list.

Expected result: No pages loads, server seems to be stuck.

CORRECTED EFFECT Do same steps, except: (...) 2.Ensure that web.maxCommitPaths=1000 (...) 6.Observe, that page loads withing a few seconds or faster. Observe that commited files list is now 1000 lines long and headers over it do contain "+more changes" and "or more" texts to indicate that something was trimmed off. (...) 8.All works as expected.

Repeat it again, but commit initial commit so that the problematic commit will be parent-less. Same effects are expected.

Repeat the test with small repository and observe that headers above commited files list should NOT contain "+ more changes" or "or more" texts.

Otherwise web interface should be visually in agreement with "master" branch.

Note: Polish translation for some of added texts is provided. I will possibly add more of them in next pull requests, since my users are Polish.

PART.2 Logging.

1.Build server. 2.Start it with --dailyLogFile Notice: "Using --dailyLogFile to override log output." on console and dump of log4j.properties file. 3.Stop it, start with "web.debugMode=true" option. Notice: "Using web.debugMode to override debug levels." on console and dump of log4j.properties file. 4.Stop it, rename data/log4j-debug-example.properties to log4j.properties, start it. Notice: "Using web.debugMode to override debug levels." on console and dump of log4j.properties file. 5.Try mixing --dailyLogFile and web.debugMode with log4j.properties. Notice different messages and prioritization of settings from log4j.properties if options do conflict.

EXPECTATIONS

I do hope that some developers would at least "cherry-pick" some of proposed changes and pin point obvious mistakes on my part. This is a first time I do actually publish fixes to open source projects, so please excuse me if I offended someone or made some wrongs.