----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52636/#review151833 -----------------------------------------------------------
Fix it, then Ship it! ambari-server/src/main/java/org/apache/ambari/server/logging/DelegatingLock.java (line 24) <https://reviews.apache.org/r/52636/#comment220406> Add some java doc for how this will be used. ambari-server/src/main/java/org/apache/ambari/server/logging/LockFactory.java (line 35) <https://reviews.apache.org/r/52636/#comment220407> Add javadoc for how this will be used. ambari-server/src/main/java/org/apache/ambari/server/logging/LockProfileDelegate.java (line 81) <https://reviews.apache.org/r/52636/#comment220417> Missing a space here, line 85, and 94 after the label. ambari-server/src/main/java/org/apache/ambari/server/logging/LockProfileDelegate.java (line 103) <https://reviews.apache.org/r/52636/#comment220418> Can we make this <= ? - Alejandro Fernandez On Oct. 7, 2016, 4:25 p.m., Attila Doroszlai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52636/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2016, 4:25 p.m.) > > > Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Sandor Magyari, > and Sebastian Toader. > > > Repository: ambari > > > Description > ------- > > 1. Create `Lock` and `ReadWriteLock` implementation for profiling and logging > lock usage > 2. Introduce `LockFactory` that creates regular or instrumented locks based > on Ambari Server configuration > 3. As sample usage, replace locks in `ClusterImpl` with the ones created by > `LockFactory` > 4. Allows on-demand dump of lock profiler summary stats as part of the debug > log for `/api/v1/clusters` requests (This is a temporary solution. Once the > factory is used in more places, the dump should be triggered by some other > request unrelated to the clusters.) > > Configuration: > > * `server.locks.profiling=true` enables usage of instrumented locks instead > of regular ones (goes in `ambari.properties`) > * > `log4j.logger.org.apache.ambari.server.controller.AmbariManagementControllerImpl=DEBUG` > required for the on-demand dump (goes in `log4j.properties`) > * `log4j.logger.org.apache.ambari.server.logging=DEBUG` enables logging of > individual "requested", "acquired", "released" events. Only the outermost > lock/unlock call is logged, reentrant calls are ignored (goes in > `log4j.properties`) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > 2e850ef > > ambari-server/src/main/java/org/apache/ambari/server/logging/DelegatingLock.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/logging/LockFactory.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/logging/LockProfileDelegate.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/logging/ProfiledLock.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/logging/ProfiledReentrantLock.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/logging/ProfiledReentrantReadWriteLock.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 2f7d6b9 > > ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java > a56ace5 > > ambari-server/src/test/java/org/apache/ambari/server/logging/DelegatingLockTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/logging/LockFactoryTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/logging/ProfiledReentrantReadWriteLockTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/52636/diff/ > > > Testing > ------- > > * Manual testing: cluster creation via blueprint both with and without > `server.locks.profiling=true` setting > * Unit tests: > Tests run: 56, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.968 sec > - in org.apache.ambari.server.configuration.ConfigurationTest > Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.044 sec > - in org.apache.ambari.server.logging.DelegatingLockTest > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.279 sec > - in org.apache.ambari.server.logging.LockFactoryTest > Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.168 sec > - in org.apache.ambari.server.logging.ProfiledReentrantReadWriteLockTest > * Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 > approved: 5012 licence. > > > Thanks, > > Attila Doroszlai > >
