Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-65398] Terminology updates for public strings. #147

Merged
merged 1 commit into from
May 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/flowgraph.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ This may seem backwards, but it enables us to freely append to the Flow Graph as
* **There are no bounds on the size of the flow graph, it may have thousands of nodes in it.** Real users with complex Pipelines will really generate graphs this size. Yes, really.
* **Repeat: there are no bounds on the size of the flow graph. This means if you use recursive function calls to iterate over the Flow Graph you will get a `StackOverFlowError`!!!** Use the `AbstractFlowScanner` implementations - they're free of stack overflows and well-tested.
* As a back of napkin estimate, most Flow Graphs fall in the 200-700 `FlowNode` range
* `GraphListener` gotcha: because the listener is invoked for each new `FlowNode`, if you implement some operation that iterates over a lot of the Flow Graph then you've just done an O(n^2) operation and it can result in very high CPU use. This can bog down a Jenkins master if not done carefully.
* `GraphListener` gotcha: because the listener is invoked for each new `FlowNode`, if you implement some operation that iterates over a lot of the Flow Graph then you've just done an O(n^2) operation and it can result in very high CPU use. This can bog down a Jenkins controller if not done carefully.
- Careful use of the methods above to iterate/find enclosing flownodes can make this much safer


# Anti-gotchas

* It may seem intimidating but the Flow Graph has a TON of useful information ripe for analysis and visualization!
* The gotchas above are mostly a problem for plugins that try to manipulate the Flow Graph or do automated analysis while Pipelines are running -- if you use the utilities and wait for Pipelines to complete then most of the problems go away.
* The actual data model is quite simple.
* The actual data model is quite simple.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class FilePathUtils {
* Note that if an administrator disconnects an agent, configures and connects an unrelated agent with the same name,
* and then this method is called on a path created against the original connection, the result may be misleading.
* @param f a file, possibly remote
* @return a node name ({@code ""} for master), if known, else null
* @return a node name ({@code ""} for the controller), if known, else null
*/
public static @CheckForNull String getNodeNameOrNull(@Nonnull FilePath f) {
return Listener.getChannelName(f.getChannel());
Expand All @@ -72,15 +72,15 @@ public class FilePathUtils {
/**
* Same as {@link #getNodeNameOrNull} but throws a diagnostic exception in case of failure.
* @param f a file, possible remote
* @return a node name ({@code ""} for master), if known
* @return a node name ({@code ""} for the controller), if known
* @throws IllegalStateException if the association to a node is unknown
*/
public static @Nonnull String getNodeName(@Nonnull FilePath f) throws IllegalStateException {
String name = getNodeNameOrNull(f);
if (name != null) {
return name;
} else {
throw new IllegalStateException("no known slave for " + f + " among " + Listener.getChannelNames());
throw new IllegalStateException("no known agent for " + f + " among " + Listener.getChannelNames());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public abstract class WorkspaceAction implements PersistentAction {
* The {@link Node#getAssignedLabels} of the node owning the workspace.
* {@link Node#getSelfLabel} should be exempted, so this set may be empty in the typical case.
* (Could be reconstructed in most cases via {@link Jenkins#getNode} on {@link #getNode},
* but not for a slave which has since been removed, common with clouds.)
* but not for an agent which has since been removed, common with clouds.)
*/
public abstract @Nonnull Set<LabelAtom> getLabels();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import javax.annotation.Nonnull;

/**
* Provides hints about just how hard we should try to protect our workflow from failures of the master.
* Provides hints about just how hard we should try to protect our workflow from failures of the controller.
* There is a trade-off between durability and performance, with higher levels carrying much higher overheads to execution.
* Storage and persistence of data should try to provide at least the specified level (may offer more).
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void notifyShutdown() {
}

/**
* A directory (on the master) where information may be persisted.
* A directory (on the controller) where information may be persisted.
* @see Run#getRootDir
*/
public abstract File getRootDir() throws IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
* </ul>
*
* <p>All APIs visit the parent nodes, walking backward from heads(inclusive) until they they hit {@link #myBlackList} nodes (exclusive) or reach the end of the DAG.
* If blackList nodes are an empty collection or null, APIs will walk to the beginning of the FlowGraph.
* Multiple blackList nodes are helpful for putting separate bounds on walking different parallel branches.
* If denyList nodes are an empty collection or null, APIs will walk to the beginning of the FlowGraph.
* Multiple denyList nodes are helpful for putting separate bounds on walking different parallel branches.
*
* <p><strong>Key Points:</strong>
* <ul><li>There are many helper methods offering syntactic sugar for the above APIs in common use cases (simpler method signatures).</li>
Expand All @@ -83,7 +83,7 @@
* <li>Scan through all nodes *just* within a block
* <ul>
* <li>Use the {@link org.jenkinsci.plugins.workflow.graph.BlockEndNode} as the head</li>
* <li>Use the {@link org.jenkinsci.plugins.workflow.graph.BlockStartNode} as its blacklist with {@link Collections#singleton(Object)}</li>
* <li>Use the {@link org.jenkinsci.plugins.workflow.graph.BlockStartNode} as its denylist with {@link Collections#singleton(Object)}</li>
* </ul></li>
* </ul>
*
Expand All @@ -99,7 +99,7 @@ public abstract class AbstractFlowScanner implements Iterable <FlowNode>, Filter

protected Collection<FlowNode> myBlackList = Collections.emptySet();

/** When checking for blacklist membership, we convert to a hashset when checking more than this many elements */
/** When checking for denylist membership, we convert to a hashset when checking more than this many elements */
protected static final int MAX_LIST_CHECK_SIZE = 5;

/** Helper: convert stop nodes to a collection that can efficiently be checked for membership, handling null if needed */
Expand Down Expand Up @@ -142,7 +142,7 @@ public boolean setup(@CheckForNull Collection<FlowNode> heads, @CheckForNull Col
}

/**
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to blacklist
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to denylist
*/
public boolean setup(@CheckForNull Collection<FlowNode> heads) {
if (heads == null) {
Expand All @@ -152,7 +152,7 @@ public boolean setup(@CheckForNull Collection<FlowNode> heads) {
}

/**
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to blacklist, and have just a single head
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to denylist, and have just a single head
*/
public boolean setup(@CheckForNull FlowNode head, @CheckForNull Collection<FlowNode> blackList) {
if (head == null) {
Expand All @@ -162,7 +162,7 @@ public boolean setup(@CheckForNull FlowNode head, @CheckForNull Collection<FlowN
}

/**
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to blacklist and have just a single head
* Helper: version of {@link #setup(Collection, Collection)} where we don't have any nodes to denylist and have just a single head
*/
public boolean setup(@CheckForNull FlowNode head) {
if (head == null) {
Expand All @@ -183,7 +183,7 @@ public boolean setup(@CheckForNull FlowNode head) {
* - {@link #reset()} has already been invoked to reset state
* - filteredHeads has already had any points in {@link #myBlackList} removed
* - none of the filteredHeads are null
* @param filteredHeads Head nodes that have been filtered against blackList
* @param filteredHeads Head nodes that have been filtered against denyList
*/
protected abstract void setHeads(@Nonnull Collection<FlowNode> filteredHeads);

Expand Down Expand Up @@ -261,19 +261,19 @@ public FlowNode findFirstMatch(@CheckForNull Collection<FlowNode> heads,

// Polymorphic methods for syntactic sugar

/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} where there is no blackList */
/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} where there is no denyList */
@CheckForNull
public FlowNode findFirstMatch(@CheckForNull Collection<FlowNode> heads, @Nonnull Predicate<FlowNode> matchPredicate) {
return this.findFirstMatch(heads, null, matchPredicate);
}

/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} where there is a single head and no blackList */
/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} where there is a single head and no denyList */
@CheckForNull
public FlowNode findFirstMatch(@CheckForNull FlowNode head, @Nonnull Predicate<FlowNode> matchPredicate) {
return this.findFirstMatch(Collections.singleton(head), null, matchPredicate);
}

/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} using {@link FlowExecution#getCurrentHeads()} to get heads and no blackList */
/** Syntactic sugar for {@link #findFirstMatch(Collection, Collection, Predicate)} using {@link FlowExecution#getCurrentHeads()} to get heads and no denyList */
@CheckForNull
public FlowNode findFirstMatch(@CheckForNull FlowExecution exec, @Nonnull Predicate<FlowNode> matchPredicate) {
if (exec != null && exec.getCurrentHeads() != null && !exec.getCurrentHeads().isEmpty()) {
Expand Down Expand Up @@ -326,13 +326,13 @@ public List<FlowNode> allNodes(@CheckForNull FlowExecution exec) {
return (exec == null) ? Collections.emptyList() : allNodes(exec.getCurrentHeads());
}

/** Syntactic sugar for {@link #filteredNodes(Collection, Collection, Predicate)} with no blackList nodes */
/** Syntactic sugar for {@link #filteredNodes(Collection, Collection, Predicate)} with no denyList nodes */
@Nonnull
public List<FlowNode> filteredNodes(@CheckForNull Collection<FlowNode> heads, @Nonnull Predicate<FlowNode> matchPredicate) {
return this.filteredNodes(heads, null, matchPredicate);
}

/** Syntactic sugar for {@link #filteredNodes(Collection, Collection, Predicate)} with a single head and no blackList nodes */
/** Syntactic sugar for {@link #filteredNodes(Collection, Collection, Predicate)} with a single head and no denyList nodes */
@Nonnull
public List<FlowNode> filteredNodes(@CheckForNull FlowNode head, @Nonnull Predicate<FlowNode> matchPredicate) {
return this.filteredNodes(Collections.singleton(head), null, matchPredicate);
Expand Down Expand Up @@ -368,7 +368,7 @@ public void visitAll(@CheckForNull Collection<FlowNode> heads, @CheckForNull Col
}
}

/** Syntactic sugar for {@link #visitAll(Collection, Collection, FlowNodeVisitor)} where we don't blacklist any nodes */
/** Syntactic sugar for {@link #visitAll(Collection, Collection, FlowNodeVisitor)} where we don't denylist any nodes */
public void visitAll(@CheckForNull Collection<FlowNode> heads, @Nonnull FlowNodeVisitor visitor) {
visitAll(heads, null, visitor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
* <p>Use case: we don't care about parallel branches or know they don't exist, we just want to walk through the top-level blocks.
*
* <p>This is the fastest and simplest way to walk a flow, because you only care about a single node at a time.
* Nuance: where there are multiple parent nodes (in a parallel block), and one is blacklisted, we'll find the first non-blacklisted one.
* Nuance: where there are multiple parent nodes (in a parallel block), and one is denylisted, we'll find the first non-denylisted one.
* @author Sam Van Oort
*/
@NotThreadSafe
Expand All @@ -63,7 +63,7 @@ protected void reset() {

/**
* {@inheritDoc}
* @param heads Head nodes that have been filtered against blackList. <strong>Do not pass multiple heads.</strong>
* @param heads Head nodes that have been filtered against denyList. <strong>Do not pass multiple heads.</strong>
*/
@Override
protected void setHeads(@Nonnull Collection<FlowNode> heads) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static final class Replacement implements SerializableOnlyOverRemoting {
private static final long serialVersionUID = 1;

private final RemoteOutputStream ros;
private final DelayBufferedOutputStream.Tuning tuning = DelayBufferedOutputStream.Tuning.DEFAULT; // load defaults on master
private final DelayBufferedOutputStream.Tuning tuning = DelayBufferedOutputStream.Tuning.DEFAULT; // load defaults on controller

Replacement(BufferedBuildListener cbl) {
this.ros = new RemoteOutputStream(new CloseProofOutputStream(cbl.out));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
* using {@link #merge} to pick up any earlier decorator in {@link StepContext#get}.
* <p>Expected to be serializable either locally or over Remoting,
* so an implementation of {@link #decorate} cannot assume that {@link JenkinsJVM#isJenkinsJVM}.
* Any master-side configuration should thus be saved into instance fields when the decorator is constructed.
* Any controller-side configuration should thus be saved into instance fields when the decorator is constructed.
* @see <a href="https://issues.jenkins-ci.org/browse/JENKINS-45693">JENKINS-45693</a>
*/
public abstract class TaskListenerDecorator implements /* TODO Remotable */ Serializable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ protected static void close(TaskListener listener) throws Exception {
logging.capture(100).record(Channel.class, Level.WARNING);
LogStorage ls = createStorage();
TaskListener overall = ls.overallListener();
overall.getLogger().println("overall from master");
overall.getLogger().println("overall from controller");
TaskListener step = ls.nodeListener(new MockNode("1"));
step.getLogger().println("step from master");
long overallPos = assertOverallLog(0, "overall from master\n<span class=\"pipeline-node-1\">step from master\n</span>", true);
long stepPos = assertStepLog("1", 0, "step from master\n", true);
step.getLogger().println("step from controller");
long overallPos = assertOverallLog(0, "overall from controller\n<span class=\"pipeline-node-1\">step from controller\n</span>", true);
long stepPos = assertStepLog("1", 0, "step from controller\n", true);
DumbSlave s = r.createOnlineSlave();
r.showAgentLogs(s, agentLoggers());
VirtualChannel channel = s.getChannel();
Expand Down