diff options
6 files changed, 45 insertions, 42 deletions
diff --git a/document/src/main/java/com/yahoo/document/BucketId.java b/document/src/main/java/com/yahoo/document/BucketId.java index 36ec53c51d3..f6d3dc34af1 100755 --- a/document/src/main/java/com/yahoo/document/BucketId.java +++ b/document/src/main/java/com/yahoo/document/BucketId.java @@ -74,9 +74,9 @@ public class BucketId implements Comparable<BucketId> { public int compareTo(BucketId other) { if (id >>> 32 == other.id >>> 32) { - if ((id & 0xFFFFFFFFl) > (other.id & 0xFFFFFFFFl)) { + if ((id & 0xFFFFFFFFL) > (other.id & 0xFFFFFFFFL)) { return 1; - } else if ((id & 0xFFFFFFFFl) < (other.id & 0xFFFFFFFFl)) { + } else if ((id & 0xFFFFFFFFL) < (other.id & 0xFFFFFFFFL)) { return -1; } return 0; @@ -97,8 +97,8 @@ public class BucketId implements Comparable<BucketId> { public long getId() { int notUsed = 64 - getUsedBits(); - long usedMask = (0xFFFFFFFFFFFFFFFFl << notUsed) >>> notUsed; - long countMask = (0xFFFFFFFFFFFFFFFFl >>> (64 - COUNT_BITS)) << (64 - COUNT_BITS); + long usedMask = (0xFFFFFFFFFFFFFFFFL << notUsed) >>> notUsed; + long countMask = (0xFFFFFFFFFFFFFFFFL >>> (64 - COUNT_BITS)) << (64 - COUNT_BITS); return id & (usedMask | countMask); } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java index f5b4920fa3f..2680ed011af 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java @@ -429,7 +429,7 @@ public class DocumentProtocol implements Protocol { * @param ctx the context whose children to merge */ public static void merge(RoutingContext ctx) { - merge(ctx, new HashSet<Integer>(0)); + merge(ctx, new HashSet<>(0)); } /** @@ -475,7 +475,7 @@ public class DocumentProtocol implements Protocol { * @return the merged Reply */ public static Reply merge(List<Reply> replies) { - return merge(replies, new HashSet<Integer>(0)).second; + return merge(replies, new HashSet<>(0)).second; } /** diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ReplyMerger.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ReplyMerger.java index a8ec53c5c97..630a8588495 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ReplyMerger.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ReplyMerger.java @@ -3,6 +3,7 @@ package com.yahoo.documentapi.messagebus.protocol; import com.yahoo.collections.Tuple2; import com.yahoo.messagebus.EmptyReply; +import com.yahoo.messagebus.Error; import com.yahoo.messagebus.Reply; /** @@ -57,15 +58,22 @@ final class ReplyMerger { return; } if (error == null) { - error = new EmptyReply(); - r.swapState(error); - return; + error = r; + } + else if (mostSevereErrorCode(r) > mostSevereErrorCode(error)) { + error.getErrors().forEach(r::addError); + error = r; } - for (int j = 0; j < r.getNumErrors(); ++j) { - error.addError(r.getError(j)); + else { + r.getErrors().forEach(error::addError); } } + private static int mostSevereErrorCode(Reply reply) { + return reply.getErrors().mapToInt(Error::getCode).max() + .orElseThrow(() -> new IllegalArgumentException(reply + " has no errors")); + } + private boolean handleReplyWithOnlyIgnoredErrors(Reply r) { if (DocumentProtocol.hasOnlyErrorsOfType(r, DocumentProtocol.ERROR_MESSAGE_IGNORED)) { if (ignore == null) { @@ -96,7 +104,7 @@ final class ReplyMerger { } private Tuple2<Integer, Reply> createEmptyReplyResult() { - return new Tuple2<>(null, (Reply)new EmptyReply()); + return new Tuple2<>(null, new EmptyReply()); } public Tuple2<Integer, Reply> mergedReply() { diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java index 157b4a6585b..11d74800a79 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java @@ -38,7 +38,7 @@ public class ReplyMergerTestCase { } @Test - public void mergingSingleReplyWithOneErrorReturnsEmptyReplyWithError() { + public void mergingSingleReplyWithOneErrorReturnsSameReplyWithError() { Reply r1 = new EmptyReply(); Error error = new Error(1234, "oh no!"); r1.addError(error); @@ -46,12 +46,12 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertNotSame(r1, ret.second); + assertSame(r1, ret.second); assertThatErrorsMatch(new Error[] { error }, ret); } @Test - public void mergingSingleReplyWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { + public void mergingSingleReplyWithMultipleErrorsReturnsSameReplyWithAllErrors() { Reply r1 = new EmptyReply(); Error errors[] = new Error[] { new Error(1234, "oh no!"), new Error(4567, "oh dear!"), @@ -62,12 +62,12 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertNotSame(r1, ret.second); + assertSame(r1, ret.second); assertThatErrorsMatch(errors, ret); } @Test - public void mergingMultipleRepliesWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { + public void mergingMultipleRepliesWithMultipleErrorsReturnsMostSevereReplyWithAllErrors() { Reply r1 = new EmptyReply(); Reply r2 = new EmptyReply(); Error errors[] = new Error[] { @@ -81,7 +81,7 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertNotSame(r1, ret.second); + assertSame(r1, ret.second); assertNotSame(r2, ret.second); assertThatErrorsMatch(errors, ret); } @@ -143,7 +143,7 @@ public class ReplyMergerTestCase { merger.merge(1, r2); Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertNotSame(r1, ret.second); + assertSame(r1, ret.second); assertNotSame(r2, ret.second); // All errors from replies with errors are included, not those that // are fully ignored. @@ -182,7 +182,7 @@ public class ReplyMergerTestCase { } @Test - // TODO: This seems wrong, and is probably a consequence of TAS being added later than this logic was written. + // TODO jonmv: This seems wrong, and is probably a consequence of TAS being implemented after reply merging. public void returnErrorDocumentReplyWhereDocWasFoundWhichIsProbablyWrong() { Error e1 = new Error(DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED, "fail"); UpdateDocumentReply r1 = new UpdateDocumentReply(); @@ -197,7 +197,7 @@ public class ReplyMergerTestCase { merger.merge(2, r3); Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertNotSame(r1, ret.second); + assertSame(r1, ret.second); assertThatErrorsMatch(new Error[] { e1 }, ret); } diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java index 89d5db62899..92e94256411 100755 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java @@ -155,16 +155,6 @@ public class PolicyTestFrame { } /** - * This is a convenience method for invoking {@link #assertMerge(Map,List,List)} with no expected value. - * - * @param replies The errors to set in the leaf node replies. - * @param expectedErrors The list of expected errors in the merged reply. - */ - public void assertMergeError(Map<String, Integer> replies, List<Integer> expectedErrors) { - assertMerge(replies, expectedErrors, null); - } - - /** * This is a convenience method for invoking {@link this#assertMerge(Map,List,List)} with no expected errors. * * @param replies The errors to set in the leaf node replies. @@ -233,10 +223,10 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipient, ErrorCode.NONE); - assertMergeOk(replies, Arrays.asList(recipient)); + assertMergeOk(replies, List.of(recipient)); replies.put(recipient, ErrorCode.TRANSIENT_ERROR); - assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); + assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR), List.of(recipient)); } /** @@ -252,28 +242,33 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, Arrays.asList(recipientOne, recipientTwo)); + assertMergeOk(replies, List.of(recipientOne, recipientTwo)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); + assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR), List.of(recipientOne)); + + replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); + replies.put(recipientTwo, ErrorCode.FATAL_ERROR); + assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR, ErrorCode.FATAL_ERROR), List.of(recipientTwo)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.TRANSIENT_ERROR); - assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR)); + assertMerge(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR), List.of(recipientOne, recipientTwo)); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMergeOk(replies, Arrays.asList(recipientOne)); + assertMergeOk(replies, List.of(recipientOne)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, Arrays.asList(recipientTwo)); + assertMergeOk(replies, List.of(recipientTwo)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMergeError(replies, Arrays.asList(DocumentProtocol.ERROR_MESSAGE_IGNORED, - DocumentProtocol.ERROR_MESSAGE_IGNORED)); + assertMerge(replies, List.of(DocumentProtocol.ERROR_MESSAGE_IGNORED, + DocumentProtocol.ERROR_MESSAGE_IGNORED), + null); // Only ignored errors specifically causes an EmptyReply. } /** diff --git a/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingTable.java b/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingTable.java index 82f409dbb44..e3410cdba7d 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingTable.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingTable.java @@ -13,8 +13,8 @@ import java.util.Map; */ public class RoutingTable { - private final Map<String, HopBlueprint> hops = new LinkedHashMap<String, HopBlueprint>(); - private final Map<String, Route> routes = new LinkedHashMap<String, Route>(); + private final Map<String, HopBlueprint> hops = new LinkedHashMap<>(); + private final Map<String, Route> routes = new LinkedHashMap<>(); /** * Creates a new routing table based on a given specification. This also verifies the integrity of the table. |