From 8a88427d02d642c0f96ad324b640758b61cbfcdd Mon Sep 17 00:00:00 2001 From: Jon Sten Date: Fri, 3 Jan 2020 15:23:56 +0100 Subject: [PATCH 1/3] Add ability to disable fingerprint recording It seems like the fingerprinting functionality has been broken for a while and causes a lot of problems, especially in large Jenkins instances. This change introduces a property which disables the functionality all together. --- .../docker/commons/fingerprint/DockerFingerprints.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java index 442cf07b..fe354fda 100644 --- a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java +++ b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java @@ -23,6 +23,7 @@ */ package org.jenkinsci.plugins.docker.commons.fingerprint; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.BulkChange; import hudson.model.Fingerprint; import hudson.model.Run; @@ -47,6 +48,9 @@ public class DockerFingerprints { private static final Logger LOGGER = Logger.getLogger(DockerFingerprints.class.getName()); + + @SuppressFBWarnings(value="MS_SHOULD_BE_FINAL", justification="mutable for scripts") + public static boolean DISABLE = Boolean.getBoolean(DockerFingerprints.class.getName() + ".DISABLE"); private DockerFingerprints() {} // no instantiation @@ -221,6 +225,9 @@ private DockerFingerprints() {} // no instantiation * Adds a new {@link ContainerRecord} for the specified image, creating necessary intermediate objects as it goes. */ public static void addRunFacet(@Nonnull ContainerRecord record, @Nonnull Run run) throws IOException { + if (DISABLE) { + return; + } String imageId = record.getImageId(); Fingerprint f = forImage(run, imageId); synchronized (f) { @@ -256,6 +263,9 @@ public static void addRunFacet(@Nonnull ContainerRecord record, @Nonnull Run run) throws IOException { + if (DISABLE) { + return; + } long timestamp = System.currentTimeMillis(); if (ancestorImageId != null) { Fingerprint f = forImage(run, ancestorImageId); From c6b86d33f9605973ffa37e010d1d8667ac7b636e Mon Sep 17 00:00:00 2001 From: Jon Sten Date: Wed, 8 Jan 2020 11:49:32 +0100 Subject: [PATCH 2/3] Add documentation and helper method for fingerprint disabling Base of review feedback. --- README.md | 7 +++++++ .../fingerprint/DockerFingerprints.java | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ec6eb5e9..2e9c2485 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,13 @@ pipeline { } ``` +## Disable fingerprinting + +Recording fingerprints are great, but for large Jenkins instances it can become a scalability and performance issue. +It is possible to disable recording of fingerprints by setting the system property +org.jenkinsci.plugins.docker.commons.fingerprint.DockerFingerprints.DISABLE equal to true. This will +not remove old fingerprints but will prevent new fingerprints from being recorded. + ## License [MIT License](http://opensource.org/licenses/MIT) diff --git a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java index fe354fda..f0625e0c 100644 --- a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java +++ b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java @@ -40,6 +40,8 @@ import javax.annotation.Nonnull; import jenkins.model.FingerprintFacet; import org.apache.commons.lang.StringUtils; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Entry point into fingerprint related functionalities in Docker. @@ -49,9 +51,14 @@ public class DockerFingerprints { private static final Logger LOGGER = Logger.getLogger(DockerFingerprints.class.getName()); + /** + * System property, which, when set to true (default false), disables + * recording of new fingerprints. + */ @SuppressFBWarnings(value="MS_SHOULD_BE_FINAL", justification="mutable for scripts") + @Restricted(NoExternalUse.class) public static boolean DISABLE = Boolean.getBoolean(DockerFingerprints.class.getName() + ".DISABLE"); - + private DockerFingerprints() {} // no instantiation /** @@ -321,4 +328,13 @@ public static void addFromFacet(@CheckForNull String ancestorImageId, @Nonnull S } } + /** + * Indicates whether recording of new fingerprints are disabled. + * + * @return true if fingerprint recording is disabled, otherwise false. + */ + public static boolean isFingerprintsDisabled() { + return DISABLE; + } + } From 753d413b635b0bc6da38de3a18908c5068dcafb2 Mon Sep 17 00:00:00 2001 From: Jon Sten Date: Thu, 9 Jan 2020 07:31:59 +0100 Subject: [PATCH 3/3] Print log message when fingerprinting is disabled --- .../plugins/docker/commons/fingerprint/DockerFingerprints.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java index f0625e0c..48d04d5a 100644 --- a/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java +++ b/src/main/java/org/jenkinsci/plugins/docker/commons/fingerprint/DockerFingerprints.java @@ -233,6 +233,7 @@ private DockerFingerprints() {} // no instantiation */ public static void addRunFacet(@Nonnull ContainerRecord record, @Nonnull Run run) throws IOException { if (DISABLE) { + LOGGER.info("Recording of fingerprints is disabled, no RUN fingerprint record will be made."); return; } String imageId = record.getImageId(); @@ -271,6 +272,7 @@ public static void addRunFacet(@Nonnull ContainerRecord record, @Nonnull Run run) throws IOException { if (DISABLE) { + LOGGER.info("Recording of fingerprints is disabled, no FROM fingerprint record will be made."); return; } long timestamp = System.currentTimeMillis();