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-39150] - API stabilization && compliance with the compatibility policy #125

6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ THE SOFTWARE.
<version>3.0.1</version>
<type>jar</type>
</dependency>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<version>1.7</version>
<type>jar</type>
</dependency>
</dependencies>

<build>
Expand Down
67 changes: 54 additions & 13 deletions src/main/java/hudson/remoting/Channel.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Represents a communication channel to the remote peer.
Expand Down Expand Up @@ -210,15 +212,15 @@ public class Channel implements VirtualChannel, IChannel, Closeable {
/**
* Number of {@link Command} objects sent to the other side.
*/
private int commandsSent;
private volatile long commandsSent;

/**
* Number of {@link Command} objects received from the other side.
*
* When a transport is functioning correctly, {@link #commandsSent} of one side
* and {@link #commandsReceived} of the other side should closely match.
*/
private int commandsReceived;
private volatile long commandsReceived;

/**
* Timestamp of the last {@link Command} object sent/received, in
Expand Down Expand Up @@ -1172,6 +1174,7 @@ public void dumpPerformanceCounters(PrintWriter w) throws IOException {
w.printf(Locale.ENGLISH, "Resource loading time=%,dms%n", resourceLoadingTime.get() / (1000 * 1000));
}

//TODO: Make public after merge into the master branch
/**
* Print the diagnostic information.
*
Expand Down Expand Up @@ -1226,17 +1229,24 @@ public void dumpPerformanceCounters(PrintWriter w) throws IOException {
* Number of RPC calls (e.g., method call through a {@linkplain RemoteInvocationHandler proxy})
* that the other side has made to us but not yet returned yet.
*
* @since 2.26.3
* @param w Output destination
* @throws IOException Error while creating or writing the channel information
*
* @since 2.62.3 - stable 2.x (restricted)
* @since TODO 3.x - public version
*/
public void dumpDiagnostics(PrintWriter w) throws IOException {
@Restricted(NoExternalUse.class)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not effectively prevent from usage in libraries, which do not invoke accmod checks in the build process. Is it fine enough?

public void dumpDiagnostics(@Nonnull PrintWriter w) throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe IOException is YAGNI, but I would like to keep it since one may override Channel implementation and start throwing something. The current implementation does not throw exceptions

w.printf("Channel %s%n",name);
w.printf(" Created=%s%n", new Date(createdAt));
w.printf(" Commands sent=%d%n", commandsSent);
w.printf(" Commands received=%d%n", commandsReceived);
w.printf(" Last command sent=%s%n", new Date(lastCommandSentAt));
w.printf(" Last command received=%s%n", new Date(lastCommandReceivedAt));
w.printf(" Pending outgoing calls=%d%n", pendingCalls.size());
w.printf(" Pending incoming calls=%d%n", pendingCalls.size());

// TODO: Update after the merge to 3.x branch, where the Hashtable is going to be replaced as a part of
// https://github.com/jenkinsci/remoting/pull/109
w.printf(" Pending calls=%d%n", pendingCalls.size());
}

/**
Expand Down Expand Up @@ -1584,17 +1594,48 @@ public static Channel current() {
return CURRENT.get();
}

// TODO: Unrestrict after the merge into the master.
// By now one may use it via the reflection logic only
/**
* Calls {@link #dumpDiagnostics(PrintWriter)} across all the active channels in this system.
* Used for diagnostics.
*
* @since 2.26.3
*/
public static void dumpDiagnosticsForAll(PrintWriter w) throws IOException {
for (Ref ref : ACTIVE_CHANNELS.values().toArray(new Ref[0])) {
Channel ch = ref.channel();
if (ch!=null)
ch.dumpDiagnostics(w);
* @param w Output destination
*
* @since 2.62.3 - stable 2.x (restricted)
* @since TODO 3.x - public version
*/
@Restricted(NoExternalUse.class)
public static void dumpDiagnosticsForAll(@Nonnull PrintWriter w) {
final Ref[] channels = ACTIVE_CHANNELS.values().toArray(new Ref[0]);
int processedCount = 0;
for (Ref ref : channels) {
// Check if we can still write the output
if (w.checkError()) {
logger.log(Level.WARNING,
String.format("Cannot dump diagnostics for all channels, because output stream encontered an error. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems spelling in my NetBeans does not work anymore :(

+ "Processed %d of %d channels, first unprocessed channel reference is %s.",
processedCount, channels.length, ref
));
break;
}

// Dump channel info if the reference still points to it
final Channel ch = ref.channel();
if (ch != null) {
try {
ch.dumpDiagnostics(w);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are performance concerned, I would recommend adding

if (w.checkError()) {
  return; 
}

as no sense continuing if PrintWriter has already suppressed the IOE

} catch (Throwable ex) {
if (ex instanceof Error) {
throw (Error)ex;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily helpful. (For ThreadDeath, yes; for LinkageError, generally no; for OutOfMemoryError, probably irrelevant.)

}
w.printf("Cannot dump diagnostics for the channel %s. %s. See Error stacktrace in system logs",
ch.getName(), ex.getMessage());
logger.log(Level.WARNING,
String.format("Cannot dump diagnostics for the channel %s", ch.getName()), ex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably send this to the print writer too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

}
}
processedCount++;
}
}

Expand Down