Skip to content

Commit

Permalink
[BEE-2537] Terminology updates for public strings. slave->agent, mast…
Browse files Browse the repository at this point in the history
…er->controller, blacklist->denylist (#147)
  • Loading branch information
kerogers-cloudbees authored May 7, 2021
1 parent fa8a6b3 commit f8641c7
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 30 deletions.
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

0 comments on commit f8641c7

Please sign in to comment.