Skip to content

Commit 7a4b3fc

Browse files
ruhan1jdcasey
ruhan1
authored and
jdcasey
committed
[1.7.x] Indy promote perf (#1205)
* Performance fixes for promotion * Throw timeout exception in runParallelAndWait * Revert default promotion groovy to use paralleledEach
1 parent af402d5 commit 7a4b3fc

11 files changed

Lines changed: 120 additions & 61 deletions

File tree

‎addons/promote/common/src/main/data/promote/rules/artifact-refs-via.groovy‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class ArtifactRefAvailability implements ValidationRule {
1515
String validate(ValidationRequest request) {
1616
def verifyStoreKeys = request.getTools().getValidationStoreKeys(request, true);
1717

18-
def errors = new ArrayList()
18+
def errors = Collections.synchronizedList(new ArrayList());
1919
def tools = request.getTools()
2020
def dc = new ModelProcessorConfig().setIncludeBuildSection(false).setIncludeManagedDependencies(false)
2121

@@ -62,17 +62,15 @@ class ArtifactRefAvailability implements ValidationRule {
6262
}
6363
})
6464

65-
synchronized (errors) {
66-
if (!found) {
67-
errors.add(String.format("%s is invalid: %s is not available via: %s",
68-
it, path, StringUtils.join(verifyStoreKeys, ", ")))
69-
}
65+
if (!found) {
66+
errors.add(String.format("%s is invalid: %s is not available via: %s",
67+
it, path, StringUtils.join(verifyStoreKeys, ", ")))
68+
}
7069

71-
if (!foundPom) {
72-
errors.add(String.format("%s is invalid: %s is not available via: %s", it,
73-
tools.toArtifactPath(target.asPomArtifact()),
74-
StringUtils.join(verifyStoreKeys, ", ")))
75-
}
70+
if (!foundPom) {
71+
errors.add(String.format("%s is invalid: %s is not available via: %s", it,
72+
tools.toArtifactPath(target.asPomArtifact()),
73+
StringUtils.join(verifyStoreKeys, ", ")))
7674
}
7775
}
7876
})

‎addons/promote/common/src/main/data/promote/rules/no-pre-existing-paths.groovy‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,15 @@ class NoPreExistingPaths implements ValidationRule {
1010
String validate(ValidationRequest request) throws PromotionValidationException {
1111
def verifyStoreKeys = request.getTools().getValidationStoreKeys(request, false);
1212

13-
def errors = new ArrayList()
13+
def errors = Collections.synchronizedList(new ArrayList());
1414
def tools = request.getTools()
1515

1616
tools.paralleledEach(request.getSourcePaths(), { it ->
1717
def aref = tools.getArtifact(it);
1818
if (aref != null) {
1919
tools.paralleledEach(verifyStoreKeys, { verifyStoreKey ->
2020
if (tools.exists(verifyStoreKey, it)) {
21-
synchronized (errors) {
22-
errors.add(String.format("%s is already available in: %s", it, verifyStoreKey))
23-
}
21+
errors.add(String.format("%s is already available in: %s", it, verifyStoreKey))
2422
}
2523
})
2624
}

‎addons/promote/common/src/main/data/promote/rules/no-snapshots.groovy‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class NoSnapshots implements ValidationRule {
1111
String validate(ValidationRequest request) {
1212
def verifyStoreKeys = request.getTools().getValidationStoreKeys(request, true)
1313

14-
def errors = new ArrayList()
14+
def errors = Collections.synchronizedList(new ArrayList());
1515
def tools = request.getTools()
1616
def dc = new ModelProcessorConfig().setIncludeBuildSection(true).setIncludeManagedPlugins(true).setIncludeManagedDependencies(true)
1717

@@ -20,9 +20,7 @@ class NoSnapshots implements ValidationRule {
2020
def ref = tools.getArtifact(it)
2121
if (ref != null) {
2222
if (!ref.getVersionSpec().isRelease()) {
23-
synchronized (errors) {
24-
errors.add(String.format("%s is a variable/snapshot version.", it))
25-
}
23+
errors.add(String.format("%s is a variable/snapshot version.", it))
2624
}
2725
}
2826

@@ -31,9 +29,7 @@ class NoSnapshots implements ValidationRule {
3129
tools.paralleledEach(relationships, { rel ->
3230
def target = rel.getTarget()
3331
if (!target.getVersionSpec().isRelease()) {
34-
synchronized (errors) {
35-
errors.add(String.format("%s uses a variable/snapshot version in: %s", target, it))
36-
}
32+
errors.add(String.format("%s uses a variable/snapshot version in: %s", target, it))
3733
}
3834
})
3935
}

‎addons/promote/common/src/main/data/promote/rules/no-version-ranges.groovy‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ class NoVersionRanges implements ValidationRule {
1010
String validate(ValidationRequest request) {
1111
def verifyStoreKeys = request.getTools().getValidationStoreKeys(request, true)
1212

13-
def errors = new ArrayList()
13+
def errors = Collections.synchronizedList(new ArrayList());
1414
def tools = request.getTools()
1515
def dc = new ModelProcessorConfig().setIncludeBuildSection(true).setIncludeManagedPlugins(true).setIncludeManagedDependencies(true)
1616

@@ -21,9 +21,7 @@ class NoVersionRanges implements ValidationRule {
2121
tools.paralleledEach(relationships, { rel ->
2222
def target = rel.getTarget()
2323
if (!target.getVersionSpec().isSingle()) {
24-
synchronized(errors) {
25-
errors.add(String.format( "%s uses a compound version in: %s", target, it))
26-
}
24+
errors.add(String.format("%s uses a compound version in: %s", target, it))
2725
}
2826
})
2927
}

‎addons/promote/common/src/main/data/promote/rules/parsable-pom.groovy‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import org.slf4j.LoggerFactory
88
class ParsablePom implements ValidationRule {
99

1010
String validate(ValidationRequest request) {
11-
def errors = new ArrayList()
11+
def errors = Collections.synchronizedList(new ArrayList());
1212
def tools = request.getTools()
1313
def logger = LoggerFactory.getLogger(ValidationRule.class)
1414
logger.info("Parsing POMs in:\n {}.", request.getSourcePaths().join("\n "))
@@ -20,9 +20,7 @@ class ParsablePom implements ValidationRule {
2020
def pom = tools.readLocalPom(it, request)
2121
}
2222
catch (Exception e) {
23-
synchronized(errors) {
24-
errors.add(String.format("%s: Failed to parse POM. Error was: %s", it, e))
25-
}
23+
errors.add(String.format("%s: Failed to parse POM. Error was: %s", it, e))
2624
}
2725
}
2826
})

‎addons/promote/common/src/main/data/promote/rules/project-artifacts.groovy‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class ProjectArtifacts implements ValidationRule {
1111

1212
String validate(ValidationRequest request) throws Exception {
1313
def classifierAndTypeSet = request.getValidationParameter("classifierAndTypeSet")
14-
def errors = new ArrayList()
14+
def errors = Collections.synchronizedList(new ArrayList());
1515

1616
if (classifierAndTypeSet != null) {
1717
def ctStrings = classifierAndTypeSet.split("\\s*,\\s*")
@@ -57,9 +57,7 @@ class ProjectArtifacts implements ValidationRule {
5757
tcs.each { tc ->
5858
logger.trace("Checking if TC: {} is in: {} for: {}", tc, entry.value, entry.key)
5959
if (!entry.value.contains(tc)) {
60-
synchronized(errors) {
61-
errors.add(String.format("%s: missing artifact with type/classifier: %s", entry.key, tc))
62-
}
60+
errors.add(String.format("%s: missing artifact with type/classifier: %s", entry.key, tc))
6361
}
6462
}
6563
}

‎addons/promote/common/src/main/data/promote/rules/project-version-pattern.groovy‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class ProjectVersionPattern implements ValidationRule {
99

1010
String validate(ValidationRequest request) throws Exception {
1111
def versionPattern = request.getValidationParameter("versionPattern")
12-
def errors = new ArrayList()
12+
def errors = Collections.synchronizedList(new ArrayList());
1313

1414
if (versionPattern != null) {
1515
def tools = request.getTools()
@@ -18,10 +18,8 @@ class ProjectVersionPattern implements ValidationRule {
1818
if (ref != null) {
1919
def vs = ref.getVersionString()
2020
if (!vs.matches(versionPattern)) {
21-
synchronized (errors) {
22-
errors.add(String.format("%s does not match version pattern: '%s' (version was: '%s')",
23-
it, versionPattern, vs))
24-
}
21+
errors.add(String.format("%s does not match version pattern: '%s' (version was: '%s')",
22+
it, versionPattern, vs))
2523
}
2624
}
2725
})

‎addons/promote/common/src/main/java/org/commonjava/indy/promote/validate/PromotionValidationTools.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,14 @@
7979
*/
8080
public class PromotionValidationTools
8181
{
82+
final Logger logger = LoggerFactory.getLogger( this.getClass() );
83+
8284
public static final String AVAILABLE_IN_STORES = "availableInStores";
8385

8486
@Deprecated
8587
public static final String AVAILABLE_IN_STORE_KEY = "availableInStoreKey";
8688

87-
private static final int DEFAULT_RULE_PARALLEL_WAIT_TIME_MINS = 10;
89+
private static final int DEFAULT_RULE_PARALLEL_WAIT_TIME_MINS = 30;
8890

8991
@Inject
9092
private ContentManager contentManager;
@@ -561,7 +563,7 @@ public <K, V> void paralleledEach( Map<K, V> map, Closure closure )
561563

562564
private <T> void runParallelAndWait( Collection<T> runCollection, Closure closure, Logger logger )
563565
{
564-
Set<T> todo = new HashSet<>( runCollection);
566+
Set<T> todo = new HashSet<>( runCollection );
565567
final CountDownLatch latch = new CountDownLatch( todo.size() );
566568
todo.forEach( e -> ruleParallelExecutor.execute( () -> {
567569
try
@@ -577,7 +579,12 @@ private <T> void runParallelAndWait( Collection<T> runCollection, Closure closur
577579

578580
try
579581
{
580-
latch.await( DEFAULT_RULE_PARALLEL_WAIT_TIME_MINS, TimeUnit.MINUTES );
582+
// true if the count reached zero and false if timeout
583+
boolean finished = latch.await( DEFAULT_RULE_PARALLEL_WAIT_TIME_MINS, TimeUnit.MINUTES );
584+
if ( !finished )
585+
{
586+
throw new RuntimeException( "Parallel execution timeout" );
587+
}
581588
}
582589
catch ( InterruptedException e )
583590
{
@@ -586,4 +593,22 @@ private <T> void runParallelAndWait( Collection<T> runCollection, Closure closur
586593
}
587594
}
588595

596+
public <T> void forEach( Collection<T> collection, Closure closure )
597+
{
598+
logger.trace( "Exe on collection {} with closure {}", collection, closure );
599+
collection.forEach( e -> closure.call( e ) );
600+
}
601+
602+
public <T> void forEach( T[] array, Closure closure )
603+
{
604+
logger.trace( "Exe on array {} with closure {}", array, closure );
605+
Arrays.asList( array ).forEach( e -> closure.call( e ) );
606+
}
607+
608+
public <K, V> void forEach( Map<K, V> map, Closure closure )
609+
{
610+
Set<Map.Entry<K, V>> entries = map.entrySet();
611+
logger.trace( "Exe on map {} with closure {}", entries, closure );
612+
entries.forEach( e -> closure.call( e ) );
613+
}
589614
}

‎addons/promote/common/src/main/java/org/commonjava/indy/promote/validate/PromotionValidator.java‎

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.commonjava.indy.promote.validate;
1717

1818
import org.apache.commons.lang.StringUtils;
19+
import org.apache.commons.lang3.ClassUtils;
1920
import org.commonjava.cdi.util.weft.DrainingExecutorCompletionService;
2021
import org.commonjava.cdi.util.weft.ExecutorConfig;
2122
import org.commonjava.cdi.util.weft.WeftExecutorService;
@@ -27,6 +28,7 @@
2728
import org.commonjava.indy.data.IndyDataException;
2829
import org.commonjava.indy.data.StoreDataManager;
2930
import org.commonjava.indy.measure.annotation.Measure;
31+
import org.commonjava.indy.metrics.IndyMetricsManager;
3032
import org.commonjava.indy.model.core.ArtifactStore;
3133
import org.commonjava.indy.model.core.RemoteRepository;
3234
import org.commonjava.indy.promote.conf.PromoteConfig;
@@ -51,7 +53,9 @@
5153
import java.util.List;
5254
import java.util.Set;
5355
import java.util.concurrent.ExecutionException;
56+
import java.util.concurrent.atomic.AtomicReference;
5457

58+
import static com.codahale.metrics.MetricRegistry.name;
5559
import static java.lang.String.format;
5660
import static org.apache.commons.lang.StringUtils.join;
5761
import static org.commonjava.indy.core.ctl.PoolUtils.detectOverloadVoid;
@@ -78,6 +82,9 @@ public class PromotionValidator
7882
@Inject
7983
private PromoteConfig config;
8084

85+
@Inject
86+
private IndyMetricsManager metricsManager;
87+
8188
@Inject
8289
@WeftManaged
8390
@ExecutorConfig( named = "promote-validation-rules-runner", threads = 20, priority = 5, loadSensitive = ExecutorConfig.BooleanLiteral.TRUE, maxLoadFactor = 400 )
@@ -90,13 +97,15 @@ protected PromotionValidator()
9097
}
9198

9299
public PromotionValidator( PromoteValidationsManager validationsManager, PromotionValidationTools validationTools,
93-
StoreDataManager storeDataMgr, DownloadManager downloadManager, WeftExecutorService validateService )
100+
StoreDataManager storeDataMgr, DownloadManager downloadManager,
101+
WeftExecutorService validateService, IndyMetricsManager metricsManager )
94102
{
95103
this.validationsManager = validationsManager;
96104
this.validationTools = validationTools;
97105
this.storeDataMgr = storeDataMgr;
98106
this.downloadManager = downloadManager;
99107
this.validateService = validateService;
108+
this.metricsManager = metricsManager;
100109
}
101110

102111
/**
@@ -228,7 +237,7 @@ public ValidationRequest validate( PromoteRequest request, ValidationResult resu
228237
}
229238
}
230239

231-
@Measure
240+
//@Measure <-- this does not work. Internal calls can never be intercepted.
232241
private void executeValidationRule( final String ruleRef, final ValidationRequest req,
233242
final ValidationResult result, final PromoteRequest request )
234243
throws PromotionValidationException
@@ -239,32 +248,68 @@ private void executeValidationRule( final String ruleRef, final ValidationReques
239248
ValidationRuleMapping rule = validationsManager.getRuleMappingNamed( ruleName );
240249
if ( rule != null )
241250
{
242-
try
251+
logger.debug( "Running promotion validation rule: {}", rule.getName() );
252+
String error = null;
253+
if ( metricsManager != null )
243254
{
244-
logger.debug( "Running promotion validation rule: {}", rule.getName() );
245-
String error = rule.getRule().validate( req );
246-
if ( StringUtils.isNotEmpty( error ) )
247-
{
248-
logger.debug( "{} failed", rule.getName() );
249-
result.addValidatorError( rule.getName(), error );
250-
}
251-
else
255+
AtomicReference<Exception> ex = new AtomicReference<>();
256+
error = metricsManager.wrapWithStandardMetrics( () -> {
257+
try
258+
{
259+
return rule.getRule().validate( req );
260+
}
261+
catch ( Exception e )
262+
{
263+
ex.set( e );
264+
return null;
265+
}
266+
}, () -> getMetricName( rule.getName() ) );
267+
268+
if ( ex.get() != null )
252269
{
253-
logger.debug( "{} succeeded", rule.getName() );
270+
throwException( ex.get(), rule, request );
254271
}
255272
}
256-
catch ( Exception e )
273+
else
257274
{
258-
if ( e instanceof PromotionValidationException )
275+
try
276+
{
277+
error = rule.getRule().validate( req );
278+
}
279+
catch ( Exception e )
259280
{
260-
throw (PromotionValidationException) e;
281+
throwException( e, rule, request );
261282
}
283+
}
262284

263-
throw new PromotionValidationException(
264-
"Failed to run validation rule: {} for request: {}. Reason: {}", e,
265-
rule.getName(), request, e );
285+
if ( StringUtils.isNotEmpty( error ) )
286+
{
287+
logger.debug( "{} failed", rule.getName() );
288+
result.addValidatorError( rule.getName(), error );
266289
}
290+
else
291+
{
292+
logger.debug( "{} succeeded", rule.getName() );
293+
}
294+
}
295+
}
296+
297+
private void throwException( Exception e, ValidationRuleMapping rule, PromoteRequest request )
298+
throws PromotionValidationException
299+
{
300+
if ( e instanceof PromotionValidationException )
301+
{
302+
throw (PromotionValidationException) e;
267303
}
304+
305+
throw new PromotionValidationException( "Failed to run validation rule: {} for request: {}. Reason: {}", e,
306+
rule.getName(), request, e );
307+
}
308+
309+
private String getMetricName( String ruleName )
310+
{
311+
String cls = ClassUtils.getAbbreviatedName( getClass().getName(), 1 );
312+
return name( cls, "rule", ruleName );
268313
}
269314

270315
private boolean needTempRepo( PromoteRequest promoteRequest )

‎addons/promote/common/src/test/java/org/commonjava/indy/promote/data/PromotionManagerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void setup()
178178
galleyParts.getMavenMetadataReader(),
179179
modelProcessor, galleyParts.getTypeMapper(),
180180
galleyParts.getTransferManager(),
181-
contentDigester ), storeManager, downloadManager, validateService );
181+
contentDigester ), storeManager, downloadManager, validateService, null );
182182

183183
PromoteConfig config = new PromoteConfig();
184184

0 commit comments

Comments
 (0)