From a5dba9c9d157762a5b7cafe4e1be98443f21c5ab Mon Sep 17 00:00:00 2001 From: Justin Lin Date: Wed, 23 Oct 2024 18:41:26 -0700 Subject: [PATCH] Use concurrent skiplist map for NettyRequest arguments --- .../main/java/com/github/ambry/rest/NettyRequest.java | 7 +++++-- .../java/com/github/ambry/rest/NettyRequestTest.java | 11 +++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java b/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java index 4ae9a6a1dc..c7967124d3 100644 --- a/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java +++ b/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java @@ -41,7 +41,7 @@ import java.util.Map; import java.util.Queue; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicBoolean; @@ -69,7 +69,10 @@ public class NettyRequest implements RestRequest { protected final HttpRequest request; protected final Channel channel; protected final NettyMetrics nettyMetrics; - protected final Map allArgs = new ConcurrentHashMap<>(); + // This allArgs map needs to be + // 1. case-insensitive, as headers and query parameters in http request should be case-insensitive + // 2. thread safe, as we might access/update the map in different threads. + protected final Map allArgs = new ConcurrentSkipListMap<>(String.CASE_INSENSITIVE_ORDER); protected final Queue requestContents = new LinkedBlockingQueue<>(); protected final ReentrantLock contentLock = new ReentrantLock(); diff --git a/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java b/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java index 740c031053..220dede9a7 100644 --- a/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java +++ b/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java @@ -754,7 +754,15 @@ private void validateRequest(NettyRequest nettyRequest, RestMethod restMethod, S for (Map.Entry e : headers) { if (!e.getKey().equalsIgnoreCase(HttpHeaderNames.COOKIE.toString())) { + // Make sure we have this key in receivedArgs assertTrue("Did not find key: " + e.getKey(), receivedArgs.containsKey(e.getKey())); + // Now make sure we can find a case-insensitive key in the request + String lowerKey = e.getKey().toLowerCase(); + assertTrue("Case insensitive key should exist for lower key" + e.getKey(), + nettyRequest.getArgs().containsKey(lowerKey)); + String upperKey = e.getKey().toUpperCase(); + assertTrue("Case insensitive key should exist for upper key" + e.getKey(), + nettyRequest.getArgs().containsKey(upperKey)); if (!keyValueCount.containsKey(e.getKey())) { keyValueCount.put(e.getKey(), 0); } @@ -769,8 +777,7 @@ private void validateRequest(NettyRequest nettyRequest, RestMethod restMethod, S assertEquals("Number of args does not match", keyValueCount.size(), receivedArgs.size()); for (Map.Entry e : keyValueCount.entrySet()) { assertEquals("Value count for key " + e.getKey() + " does not match", - e.getValue() == 0 ? 1 : e.getValue().intValue(), - receivedArgs.get(e.getKey()).size()); + e.getValue() == 0 ? 1 : e.getValue().intValue(), receivedArgs.get(e.getKey()).size()); } assertEquals("Auto-read is in an invalid state",