diff options
6 files changed, 42 insertions, 45 deletions
diff --git a/document/src/main/java/com/yahoo/document/BucketId.java b/document/src/main/java/com/yahoo/document/BucketId.java index f6d3dc34af1..36ec53c51d3 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 2680ed011af..f5b4920fa3f 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<>(0)); + merge(ctx, new HashSet<Integer>(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<>(0)).second; + return merge(replies, new HashSet<Integer>(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 630a8588495..a8ec53c5c97 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,7 +3,6 @@ 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; /** @@ -58,22 +57,15 @@ final class ReplyMerger { return; } if (error == null) { - error = r; - } - else if (mostSevereErrorCode(r) > mostSevereErrorCode(error)) { - error.getErrors().forEach(r::addError); - error = r; + error = new EmptyReply(); + r.swapState(error); + return; } - else { - r.getErrors().forEach(error::addError); + for (int j = 0; j < r.getNumErrors(); ++j) { + error.addError(r.getError(j)); } } - 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) { @@ -104,7 +96,7 @@ final class ReplyMerger { } private Tuple2<Integer, Reply> createEmptyReplyResult() { - return new Tuple2<>(null, new EmptyReply()); + return new Tuple2<>(null, (Reply)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 11d74800a79..157b4a6585b 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 mergingSingleReplyWithOneErrorReturnsSameReplyWithError() { + public void mergingSingleReplyWithOneErrorReturnsEmptyReplyWithError() { 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); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertThatErrorsMatch(new Error[] { error }, ret); } @Test - public void mergingSingleReplyWithMultipleErrorsReturnsSameReplyWithAllErrors() { + public void mergingSingleReplyWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { 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); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertThatErrorsMatch(errors, ret); } @Test - public void mergingMultipleRepliesWithMultipleErrorsReturnsMostSevereReplyWithAllErrors() { + public void mergingMultipleRepliesWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { 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); - assertSame(r1, ret.second); + assertNotSame(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); - assertSame(r1, ret.second); + assertNotSame(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 jonmv: This seems wrong, and is probably a consequence of TAS being implemented after reply merging. + // TODO: This seems wrong, and is probably a consequence of TAS being added later than this logic was written. 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); - assertSame(r1, ret.second); + assertNotSame(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 92e94256411..89d5db62899 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,6 +155,16 @@ 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. @@ -223,10 +233,10 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipient, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipient)); + assertMergeOk(replies, Arrays.asList(recipient)); replies.put(recipient, ErrorCode.TRANSIENT_ERROR); - assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR), List.of(recipient)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); } /** @@ -242,33 +252,28 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipientOne, recipientTwo)); + assertMergeOk(replies, Arrays.asList(recipientOne, recipientTwo)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.NONE); - 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)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.TRANSIENT_ERROR); - assertMerge(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR), List.of(recipientOne, recipientTwo)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR)); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMergeOk(replies, List.of(recipientOne)); + assertMergeOk(replies, Arrays.asList(recipientOne)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipientTwo)); + assertMergeOk(replies, Arrays.asList(recipientTwo)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMerge(replies, List.of(DocumentProtocol.ERROR_MESSAGE_IGNORED, - DocumentProtocol.ERROR_MESSAGE_IGNORED), - null); // Only ignored errors specifically causes an EmptyReply. + assertMergeError(replies, Arrays.asList(DocumentProtocol.ERROR_MESSAGE_IGNORED, + DocumentProtocol.ERROR_MESSAGE_IGNORED)); } /** 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 e3410cdba7d..82f409dbb44 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<>(); - private final Map<String, Route> routes = new LinkedHashMap<>(); + private final Map<String, HopBlueprint> hops = new LinkedHashMap<String, HopBlueprint>(); + private final Map<String, Route> routes = new LinkedHashMap<String, Route>(); /** * Creates a new routing table based on a given specification. This also verifies the integrity of the table. |