Jenkins 13493 - Automatic git cache maintenance #1277

Hrushi20 · 2022-05-31T22:15:45Z

JENKINS-13493 - Automatic git cache maintenance

This PR adds git maintenance to the Jenkins controller to optimize the git caches on the controller.

Please refer to Project Page for reference and design:
https://www.jenkins.io/projects/gsoc/2022/projects/automatic-git-cache-maintenance/

Checklist

Types of changes

New feature (non-breaking change which adds functionality)

Hrushi20 · 2022-06-19T20:57:21Z

Thank You @MarkEWaite for fixing those tests.

rishabhBudhouliya · 2022-07-12T19:48:40Z

src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java

@@ -1253,6 +1274,7 @@ protected static File getCacheDir(String cacheEntry, boolean createDirectory) {
        if (!cacheDir.isDirectory()) {
            if (createDirectory) {
                boolean ok = cacheDir.mkdirs();
+                cacheEntries.add(cacheEntry);


would this not add invalid cache directory entry to the static map?

Suggested change

cacheEntries.add(cacheEntry);

if (ok) {

cacheEntries.add(cacheEntry);

} else {

LOGGER.log(Level.WARNING, "Failed mkdirs of {0}", cacheDir);

}

Fixed this issue. Thank You. My apologies for not observing it.

rishabhBudhouliya · 2022-07-12T19:56:56Z

src/main/java/jenkins/plugins/git/maintenance/MaintenanceTaskConfiguration.java

+
+    public static String checkSanity(String cron) throws ANTLRException {
+       try {
+           CronTab cronTab = new CronTab(cron.trim());


can the input string cron ever be null? It could be a potential NPE.

The cron string will not be null. If the user leave the text field blank in the UI, an empty string is passed to this method.

Needs a NonNull annotation to declare to spotbugs that the argument should never be allowed to be null.

src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java

rishabhBudhouliya · 2022-07-26T18:06:45Z

src/main/java/jenkins/plugins/git/maintenance/TaskExecutor.java

+
+                        lock.lock();
+                        LOGGER.log(Level.FINE, "Cache " + cacheFile.getName() + " locked.");
+                        executeMaintenanceTask(gitClient, taskType);


I have some concerns over this locking mechanism.

running a git maintenance task can be time intensive, locking a thread while others are waiting to acquire the lock might increase the number of waiting threads (need to take a jvm thread dump to analyse and confirm this)

we should try to reduce the scope of locking instead of locking the whole git maintenance process, i.e, identify the operations that need locking and only apply it conditionally

another strategy to use locks, lock.tryLock(time interval). This allows the waiting threads to wait for a certain time and move on if the subjected lock to be acquired is not free.

rishabhBudhouliya · 2022-07-26T18:11:32Z

src/main/java/jenkins/plugins/git/maintenance/TaskScheduler.java

+
+            List<Task> configuredTasks = config.getMaintenanceTasks();
+            addTasksToQueue(configuredTasks);
+            // Option of Using the same thread for executing more maintenance task, or create a new thread the next minute and execute the maintenance task.


This is interesting, would like to discuss this more in our weekly sync.

MarkEWaite · 2022-09-07T02:10:54Z

pom.xml

+    <dependency>
+      <groupId>com.thoughtworks.xstream</groupId>
+      <artifactId>xstream</artifactId>
+      <version>1.4.19</version>
+    </dependency>


The xstream API is already provided by Jenkins core without requiring the inclusion of a dependency. This needs to be removed. It causes several additional transitive dependencies to be included that should not be in the git plugin.

Suggested change

<dependency>

<groupId>com.thoughtworks.xstream</groupId>

<artifactId>xstream</artifactId>

<version>1.4.19</version>

</dependency>

MarkEWaite · 2022-09-07T02:47:26Z

src/main/java/jenkins/plugins/git/maintenance/MaintenanceTaskConfiguration.java

+
+    public static String checkSanity(String cron) throws ANTLRException {
+       try {
+           CronTab cronTab = new CronTab(cron.trim());


Needs a NonNull annotation to declare to spotbugs that the argument should never be allowed to be null.

src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java

MarkEWaite · 2022-09-07T03:17:15Z

src/main/resources/jenkins/plugins/git/maintenance/MaintenanceUI/help-commit-graph.html

@@ -0,0 +1,3 @@
+<div>


@MarkEWaite needs to find the technique to add the help icon to the user interface. Users will expect to view help for the various maintenance commands.

MarkEWaite · 2022-10-22T06:43:36Z

@Hrushi20 it appears there are testing failing in the CI job that were not failing before. I'm on vacation and won't be able to investigate for several weeks. If you have time over the next few weeks, it would be very nice if you could investigate the test failures.

Hrushi20 · 2022-10-24T15:02:04Z

Sure @MarkEWaite I'll look into it. Enjoy your vacation.

Hrushi20 · 2022-10-25T20:46:20Z

Hello @MarkEWaite, I have gone through the failed tests. I realized that the below code is not creating a cache on the Jenkins controller.

        sampleRepo1.init();
        sampleRepo1.git("checkout", "-b", "bug/JENKINS-42817");
        sampleRepo1.write("file", "modified");
        sampleRepo1.git("commit", "--all", "--message=dev");

        SCMFileSystem fs = SCMFileSystem.of(j.createFreeStyleProject(), new GitSCM(GitSCM.createRepoList(sampleRepo1.toString(), null), Collections.singletonList(new BranchSpec("*/bug/JENKINS-42817")), null, null, Collections.emptyList()));

Before I could access the caches, but now I am not able to create caches.
Also, Java 8 clears all the tests. Java 11 and Java 17 fails this test.

Is there a way to create a MultibranchPipeline so that it handle creating the caches.

Hrushi20 · 2022-12-20T14:21:58Z

Hello @MarkEWaite, can you help me fix these failing tests. The problem I am facing right now is that I am not able to create fake caches during tests.

MarkEWaite · 2022-12-20T14:26:17Z

Hello @MarkEWaite, can you help me fix these failing tests. The problem I am facing right now is that I am not able to create fake caches during tests.

I won't be able to help with these tests until after I've completed the release of git client plugin 4.0.0 and git plugin 5.0.0. Those releases are getting all the time that I can allocate to the git plugin and git client plugin.

Hrushi20 · 2023-07-30T17:56:25Z

Hey @MarkEWaite, incremental repack is failing. I'm getting task timeout error. Can you help me with that. I fixed all the other tests. Looking forward to getting this feature merged.

remove unwanted test

Provided by Jenkins core, no need for a separate dependency. Cause several other transitive dependencies to be included unnecessarily.

Let spotbugs help with checks for null pointer exceptions

See CONTRIBUTING file for rationale

Provides maintenance API

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 · 2023-12-10T07:47:33Z

Hey @MarkEWaite, I've fixed all the Unit test Issues. I ran a git rebase and the code is up to date with latest master. There are 2 spotbug issues, I am facing. Should I use the Spotbug flag to neglect such an issue? Looking forward to getting this merged.

Hrushi20 · 2024-02-03T03:47:32Z

Hey @MarkEWaite, can we get this code merged?

github-actions bot added the dependencies Dependency related change label May 31, 2022

github-actions bot added the test label Jun 8, 2022

Hrushi20 closed this Jun 10, 2022

Hrushi20 reopened this Jun 12, 2022

MarkEWaite changed the title ~~Jenkins 13493~~ Jenkins 13493 - Automatic git cache maintenance Jun 20, 2022

Hrushi20 closed this Jun 26, 2022

Hrushi20 reopened this Jun 26, 2022

Hrushi20 closed this Jul 8, 2022

Hrushi20 reopened this Jul 8, 2022

rishabhBudhouliya reviewed Jul 12, 2022

View reviewed changes

src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java Outdated Show resolved Hide resolved

rishabhBudhouliya reviewed Jul 12, 2022

View reviewed changes

src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java Outdated Show resolved Hide resolved

rishabhBudhouliya reviewed Jul 26, 2022

View reviewed changes

MarkEWaite added enhancement Improvement or new feature and removed dependencies Dependency related change test labels Sep 3, 2022

github-actions bot added dependencies Dependency related change test labels Sep 3, 2022

MarkEWaite requested changes Sep 7, 2022

View reviewed changes

MarkEWaite reviewed Sep 7, 2022

View reviewed changes

github-actions bot removed the test label Jul 30, 2023

Hrushi20 and others added 26 commits December 10, 2023 12:49

UI fixes

bb21cab

added format for execution time in UI

cffbe4b

Removed git maintenance legacy api

4da3432

removed gitVersionAtLeast method

8240acf

remove unwanted test

Remove xstream dependency, not required

e21ea2b

Provided by Jenkins core, no need for a separate dependency. Cause several other transitive dependencies to be included unnecessarily.

Annotate argument as NonNull

db03e75

Let spotbugs help with checks for null pointer exceptions

Remove reordering of imports

c0d3e98

See CONTRIBUTING file for rationale

Use correct case in middle of sentence

2bd9213

Use heading text consistent with Jenkins configuration

f84a4ee

updated execution status using gitClient

c889e96

storing size of cache in Bytes

55ed559

displaying cacheSize in MB only

5aa22bc

update git-client incremental version

01e27c3

Require git client plugin 3.12.0

83044e5

Provides maintenance API

Skipping caches whose size less than 10 MB

3c1a64a

Minor formatting change

cc5c990

Only log cache reads at FINE level

215f75c

Remove System.out.println calls

c949c45

Update test TaskExecutor

5499217

Update TaskSchedulerTest

7245206

updated GitMaintenanceSCMTest

f728ae1

Replace LinkedList with ArrayList

8d72be3

attempt to fix incremental-repack test

3c3a39c

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

extend lifetime of execution thread unit test

026aa92

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Fixed spotbug issue, update pom.xml

e266d16

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Fix javadoc error

c0ff128

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 force-pushed the jenkins-13493 branch from 2214dbc to c0ff128 Compare December 10, 2023 07:19

MarkEWaite removed the dependencies Dependency related change label Apr 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jenkins 13493 - Automatic git cache maintenance #1277

Jenkins 13493 - Automatic git cache maintenance #1277

Hrushi20 commented May 31, 2022 •

edited

Hrushi20 commented Jun 19, 2022

rishabhBudhouliya Jul 12, 2022

Hrushi20 Jul 14, 2022

rishabhBudhouliya Jul 12, 2022

Hrushi20 Jul 13, 2022

MarkEWaite Sep 7, 2022

rishabhBudhouliya Jul 26, 2022

rishabhBudhouliya Jul 26, 2022

MarkEWaite Sep 7, 2022

MarkEWaite Sep 7, 2022

MarkEWaite Sep 7, 2022

MarkEWaite commented Oct 22, 2022

Hrushi20 commented Oct 24, 2022 •

edited

Hrushi20 commented Oct 25, 2022 •

edited

Hrushi20 commented Dec 20, 2022

MarkEWaite commented Dec 20, 2022

Hrushi20 commented Jul 30, 2023 •

edited

Hrushi20 commented Dec 10, 2023

Hrushi20 commented Feb 3, 2024 •

edited

-                cacheEntries.add(cacheEntry);
+                if (ok) {
+                   cacheEntries.add(cacheEntry);
+                } else {
+                   LOGGER.log(Level.WARNING, "Failed mkdirs of {0}", cacheDir);
+                }

Jenkins 13493 - Automatic git cache maintenance #1277

Are you sure you want to change the base?

Jenkins 13493 - Automatic git cache maintenance #1277

Conversation

Hrushi20 commented May 31, 2022 • edited