[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

Dohbedoh · 2022-05-09T00:10:36Z

Only use channel.callAsync (through the CallAsyncWrapper) if retrieving data on remote node. When retrieving data on the built-in node, just execute the callable.
updated CI to build use containers, and build on JDK 11 and JDK 17.

I am hoping that this fix the flakyness with JDK 17 as we remove the remoting overhead for this use case. See the following for reference:

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

…n node

basil

Very nice!

basil · 2022-05-09T00:15:25Z

src/main/java/com/cloudbees/jenkins/support/AsyncResultCache.java

+        Future<V> future;
+        // If running in on the built-in node, no need to use the CallAsyncWrapper or be subjected 
+        // to the REMOTE_OPERATION_TIMEOUT_MS
+        if (node instanceof Jenkins) {


Might be safer to call JenkinsJVM.isJenkinsJVM() if this might run on an agent? Not sure if node instanceof Jenkins will trigger classloading of the main Jenkins class on agents (which would be undesirable).

The AsyncResultCache actually always executes on the controller JVM. It accepts the node and the callable and then kind of delegate the asynchronous execution and just retrieve the result. So here we check if the node in which we are going to execute the callable is the controller or not.
Maybe I can rephrase my comment.

Dohbedoh · 2022-05-09T00:29:22Z

Still failing with the timeout though 🤔

I can't reproduce wth JDK 17 locally on my machine (MBP 2019 32 GB RAM).
I can reproduce when running in the jenkinsciinfra/inbound-agent-maven@sha256:910e22e3c2bf78f3a5dbea05cc33f315ecdeb0e3c0bb98683a573640df85dbb7 container.

I captured a thread dump programmatically on TimeoutException and we seem to spend time on com.cloudbees.jenkins.support.filter.InetAddressContentFilter.filter:

"Computer.threadPoolForRemoting [#1]" 
   java.lang.Thread.State: RUNNABLE
        at java.base@17.0.2/java.util.regex.Pattern$BmpCharPredicate$$Lambda$66/0x00000001000ec710.is(Unknown Source)
        at java.base@17.0.2/java.util.regex.Pattern$BmpCharPredicate.lambda$union$2(Pattern.java:5626)
        at java.base@17.0.2/java.util.regex.Pattern$BmpCharPredicate$$Lambda$66/0x00000001000ec710.is(Unknown Source)
        at java.base@17.0.2/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:3954)
        at java.base@17.0.2/java.util.regex.Pattern$Curly.match(Pattern.java:4357)
        at java.base@17.0.2/java.util.regex.Pattern$GroupHead.match(Pattern.java:4789)
        at java.base@17.0.2/java.util.regex.Pattern$Loop.matchInit(Pattern.java:4920)
        at java.base@17.0.2/java.util.regex.Pattern$Prolog.match(Pattern.java:4844)
        at java.base@17.0.2/java.util.regex.Pattern$Branch.match(Pattern.java:4734)
        at java.base@17.0.2/java.util.regex.Pattern$GroupHead.match(Pattern.java:4789)
        at java.base@17.0.2/java.util.regex.Pattern$Branch.match(Pattern.java:4734)
        at java.base@17.0.2/java.util.regex.Pattern$GroupHead.match(Pattern.java:4789)
        at java.base@17.0.2/java.util.regex.Pattern$Bound.match(Pattern.java:5387)
        at java.base@17.0.2/java.util.regex.Pattern$Start.match(Pattern.java:3608)
        at java.base@17.0.2/java.util.regex.Matcher.search(Matcher.java:1728)
        at java.base@17.0.2/java.util.regex.Matcher.find(Matcher.java:745)
        at app//com.cloudbees.jenkins.support.filter.InetAddressContentFilter.filter(InetAddressContentFilter.java:70)
        at app//com.cloudbees.jenkins.support.filter.AllContentFilters.filter(AllContentFilters.java:43)
        at app//com.cloudbees.jenkins.support.filter.ContentFilter.filter(ContentFilter.java:77)
        at app//com.cloudbees.jenkins.support.impl.FileDescriptorLimit.listAllOpenFileDescriptors(FileDescriptorLimit.java:179)
        at app//com.cloudbees.jenkins.support.impl.FileDescriptorLimit.access$300(FileDescriptorLimit.java:43)
        at app//com.cloudbees.jenkins.support.impl.FileDescriptorLimit$GetUlimit.call(FileDescriptorLimit.java:139)
        at app//com.cloudbees.jenkins.support.impl.FileDescriptorLimit$GetUlimit.call(FileDescriptorLimit.java:117)
        at app//com.cloudbees.jenkins.support.AsyncResultCache.lambda$get$0(AsyncResultCache.java:56)
        at app//com.cloudbees.jenkins.support.AsyncResultCache$$Lambda$449/0x0000000100707b50.call(Unknown Source)
        at app//jenkins.util.ContextResettingExecutorService$2.call(ContextResettingExecutorService.java:46)
        at app//jenkins.security.ImpersonatingExecutorService$2.call(ImpersonatingExecutorService.java:80)
        at java.base@17.0.2/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base@17.0.2/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base@17.0.2/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base@17.0.2/java.lang.Thread.run(Thread.java:833)

Dohbedoh · 2022-05-09T06:14:49Z

That regexp for IPv6 sure is taking a toll.. I have tested others like the one from the Regular Expression Cookbook:

private static final String IPv6 = "(?:(?:(?:[A-Fa-f0-9]{1,4}:){6}|(?=(?:[A-Fa-f0-9]{0,4}:){0,6}(?:[0-9]{1,3}\\.){3}[0-9]{1,3}(?![:.\\w]))(([0-9A-Fa-f]{1,4}:){0,5}|:)((:[0-9A-Fa-f]{1,4}){1,5}:|:))(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)|(?:[A-Fa-f0-9]{1,4}:){7}[A-Fa-f0-9]{1,4}|(?=(?:[A-Fa-f0-9]{0,4}:){0,7}[A-Fa-f0-9]{0,4}(?![:.\\w]))(([0-9A-Fa-f]{1,4}:){1,7}|:)((:[0-9A-Fa-f]{1,4}){1,7}|:))(?![:.\\w])";

But it is also kind of slow.

The one from http://www.java2s.com/example/java/java.util.regex/is-ipv6-address-by-regex.html seems promising and simpler:

    private static final String IPv6_STANDARD = "(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}";
    private static final String IPv6_COMPRESSED = "((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)";

Will do more test. but this is most likely the better solution to the problem we are trying to solve.

Dohbedoh · 2022-05-10T06:01:17Z

I realized that regexp for IPv6 addresses at https://github.com/jenkinsci/support-core-plugin/blob/1162.vb_b_e5198c6b_22/src/main/java/com/cloudbees/jenkins/support/filter/InetAddressContentFilter.java#L61 is not even capturing all IPv6. It misses the compressed and mixed addresses... And above all, it is terribly slow...

I am updating tests that reflect this. And will then update a solution that seem promising.

Dohbedoh · 2022-05-10T06:31:19Z

Those tests shows that the current regex does not find valid IPv6 compressed addresses like:

Other found falsifying value(s) :- 
::0:0:0:0:0:1:0
::0:0:0:0:2:1:0
::0:0:0:0:2:4:0
::0:0:0:0:d:4:0
::0:0:0:0:d:9:0
::0:0:0:0:d:f:0
::0:0:0:0:d:16:0
::0:0:0:0:e:16:0
::0:0:0:0:f:16:0
::0:0:0:0:f:3c:0

But also notice how the test take seconds in JDK 11 and more than a minute on JDK 17..

Now pushing an improved regex to solve all this..

Dohbedoh · 2022-05-10T07:14:16Z

Now all the tests run in <1s. And the tests covers many more IPv6 syntaxes.
cc @basil for quick review

basil

Very nice!

Dohbedoh · 2022-05-11T01:52:01Z

I also tested this in different environments and that looks good. And seems to be reducing the generation time when anonymization is on.

Dohbedoh added 2 commits May 9, 2022 10:03

[JENKINS-68339] Skip remoting stack and timeout if running on built-i…

9f73e9c

…n node

[JENKINS-68339][ci] Remove JDK 8 CI and add JDK 17 CI

edad3e8

Dohbedoh requested a review from basil May 9, 2022 00:10

Dohbedoh requested a review from a team as a code owner May 9, 2022 00:10

basil approved these changes May 9, 2022

View reviewed changes

Dohbedoh marked this pull request as draft May 9, 2022 02:17

Dohbedoh added 2 commits May 9, 2022 17:20

[JENKINS-68339] Improve comment

e0290c8

[JENKINS-68339] Add complete tests for IPv6 filter

d60afd9

Dohbedoh changed the title ~~[JENKINS-68339] Skip remoting stack and timeout if running on built-in node~~ [JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex May 10, 2022

[JENKINS-68339] Improve Regex for IPv6 addresses detection

a7b4ea1

Dohbedoh marked this pull request as ready for review May 10, 2022 08:04

basil approved these changes May 10, 2022

View reviewed changes

Dohbedoh added enhancement chore labels May 11, 2022

Dohbedoh merged commit a1fcf85 into jenkinsci:master May 11, 2022

Dohbedoh deleted the JENKINS-68339 branch May 11, 2022 01:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

Dohbedoh commented May 9, 2022 •

edited

basil left a comment

basil May 9, 2022

Dohbedoh May 9, 2022 •

edited

Dohbedoh commented May 9, 2022 •

edited

Dohbedoh commented May 9, 2022 •

edited

Dohbedoh commented May 10, 2022

Dohbedoh commented May 10, 2022 •

edited

Dohbedoh commented May 10, 2022 •

edited

basil left a comment

Dohbedoh commented May 11, 2022

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

Conversation

Dohbedoh commented May 9, 2022 • edited

basil left a comment

Choose a reason for hiding this comment

basil May 9, 2022

Choose a reason for hiding this comment

Dohbedoh May 9, 2022 • edited

Choose a reason for hiding this comment

Dohbedoh commented May 9, 2022 • edited

Dohbedoh commented May 9, 2022 • edited

Dohbedoh commented May 10, 2022

Dohbedoh commented May 10, 2022 • edited

Dohbedoh commented May 10, 2022 • edited

basil left a comment

Choose a reason for hiding this comment

Dohbedoh commented May 11, 2022

Dohbedoh commented May 9, 2022 •

edited

Dohbedoh May 9, 2022 •

edited

Dohbedoh commented May 9, 2022 •

edited

Dohbedoh commented May 9, 2022 •

edited

Dohbedoh commented May 10, 2022 •

edited

Dohbedoh commented May 10, 2022 •

edited