Skip to content

BITMAG-991: ContributorResponseStatus logging lacks context #28

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.

Sign up for GitHub

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

Merged
Bohlski merged 1 commit into from
Aug 16, 2022
Merged

BITMAG-991: ContributorResponseStatus logging lacks context #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions ...-client/src/main/java/org/bitrepository/client/conversation/GeneralConversationState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public abstract class GeneralConversationState implements ConversationState {
private final Logger log = LoggerFactory.getLogger(getClass());
/** The scheduler used for timeout checks. */
private static final ScheduledExecutorService timer = Executors.newScheduledThreadPool(1,
new DefaultThreadFactory("ConversationState-Timeout-", Thread.NORM_PRIORITY));
new DefaultThreadFactory("ConversationState-Timeout-", Thread.NORM_PRIORITY));
private ScheduledFuture<?> scheduledTimeout;
/** For response bookkeeping */
private final ContributorResponseStatus responseStatus;
Expand All @@ -63,7 +63,7 @@ protected GeneralConversationState(Collection<String> expectedContributors) {
}

/**
* Startes the state by: <ol>
* Starts the state by: <ol>
* <li>Scheduling a timeout.</li>
* <li>Sends the request which triggers the responses for this state.</li>
* </ol>
Expand Down Expand Up @@ -107,9 +107,9 @@ public final void handleMessage(Message message) {
}

try {
if(processMessage(response)) {
if (processMessage(response)) {
responseStatus.responseReceived(response);
if (responseStatus.haveAllComponentsResponded()) {
if (responseStatus.haveAllComponentsResponded(response)) {
scheduledTimeout.cancel(true);
changeState();
}
Expand Down
24 changes: 17 additions & 7 deletions ...c/main/java/org/bitrepository/client/conversation/selector/ContributorResponseStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ public ContributorResponseStatus(Collection<String> componentsWhichShouldRespond
public final void responseReceived(MessageResponse response) {
if (MessageUtils.isEndMessageForPrimitive(response)) {
String componentID = response.getFrom();
log.debug("Received response from: " + componentID);
log.debug("Received response from: {} ({})", componentID, MessageUtils.createMessageIdentifier(response));
if (componentsWithOutstandingResponse.contains(componentID)) {
componentsWithOutstandingResponse.remove(componentID);
} else if (!componentsWithOutstandingResponse.contains(componentID) && componentsWhichShouldRespond.contains(componentID)) {
log.debug("Received more than one response from component " + componentID);
} else if (!componentsWithOutstandingResponse.contains(componentID) &&
componentsWhichShouldRespond.contains(componentID)) {
log.debug("Received more than one response from component {}", componentID);
} else {
log.debug("Received response from irrelevant component " + componentID);
log.debug("Received response from irrelevant component {}", componentID);
}
}
}
Expand All @@ -83,12 +84,21 @@ public Collection<String> getOutstandingComponents() {
/**
* @return true if all components have responded. False otherwise
*/
public final boolean haveAllComponentsResponded() {
log.debug("Expected contributors: " + componentsWhichShouldRespond + ", components that have not answered: " +
componentsWithOutstandingResponse);
public final boolean haveAllComponentsResponded(MessageResponse response) {
logContributorsStatus(response);
return componentsWithOutstandingResponse.isEmpty();
}

/**
* Logs status of expected- and outstanding contributors.
* @param response The received response.
*/
private void logContributorsStatus(MessageResponse response) {
log.debug("Expected contributors: {}, components that have not answered: {} ({})",
componentsWhichShouldRespond, componentsWithOutstandingResponse,
MessageUtils.createMessageIdentifier(response));
}

/**
* @return The set of components which should respond to the identification request.
*/
Expand Down
2 changes: 1 addition & 1 deletion ...ory-client/src/test/java/org/bitrepository/modify/putfile/PutFileClientComponentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ public void fileExistsOnPillarDifferentChecksumFromPillar() throws Exception {
@Test(groups={"regressiontest"})
public void sameFileExistsOnOnePillar() throws Exception {
addDescription("Tests that PutClient handles the presence of a file correctly, when the pillar " +
"returns a checksum equal the file being put (idempotent). ");
"returns a checksum equal the file being put (idempotent).");
TestEventHandler testEventHandler = new TestEventHandler(testEventManager);
PutFileClient putClient = createPutFileClient();

Expand Down
11 changes: 7 additions & 4 deletions bitrepository-core/src/main/java/org/bitrepository/protocol/utils/MessageUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public static boolean isPositiveIdentifyResponse(MessageResponse response) {
*/
public static boolean isPositiveProgressResponse(MessageResponse response) {
ResponseCode responseCode = response.getResponseInfo().getResponseCode();
return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS) ||
return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) ||
responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS) ||
responseCode.equals(ResponseCode.OPERATION_PROGRESS);
}

Expand All @@ -66,7 +67,8 @@ public static boolean isPositiveProgressResponse(MessageResponse response) {
*/
public static boolean isIdentifyResponse(MessageResponse response) {
ResponseCode responseCode = response.getResponseInfo().getResponseCode();
return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || responseCode.equals(ResponseCode.IDENTIFICATION_NEGATIVE);
return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) ||
responseCode.equals(ResponseCode.IDENTIFICATION_NEGATIVE);
}

/**
Expand All @@ -82,12 +84,13 @@ public static boolean isIdentifyRequest(Message message) {

/**
* @param response the supplied message
* @return whether the supplied message can be considered a end response for a primitive, emg. ends a series of
* @return whether the supplied message can be considered an end response for a primitive, i.e. it ends a series of
* identify or operation responses.
*/
public static boolean isEndMessageForPrimitive(MessageResponse response) {
ResponseCode responseCode = response.getResponseInfo().getResponseCode();
return !(responseCode.equals(ResponseCode.OPERATION_PROGRESS) || responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS));
return !(responseCode.equals(ResponseCode.OPERATION_PROGRESS) ||
responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS));
}

/**
Expand Down