discard and log error about data items that are too big to store (#16)

Author: eli-darkly

README.md

@@ -68,6 +68,22 @@ The LaunchDarkly SDK has a standard caching mechanism for any persistent data st
             )
             .build();
 
+## Data size limitation
+
+DynamoDB has [a 400KB limit](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/ServiceQuotas.html#limits-items) on the size of any data item. For the LaunchDarkly SDK, a data item consists of the JSON representation of an individual feature flag or segment configuration, plus a few smaller attributes. You can see the format and size of these representations by querying `https://sdk.launchdarkly.com/flags/latest-all` and setting the `Authorization` header to your SDK key.
+
+Most flags and segments won't be nearly as big as 400KB, but they could be if for instance you have a long list of user keys for individual user targeting. If the flag or segment representation is too large, it cannot be stored in DynamoDB. To avoid disrupting storage and evaluation of other unrelated feature flags, the SDK will simply skip storing that individual flag or segment, and will log a message (at ERROR level) describing the problem. For example:
+
+```
+    The item "my-flag-key" in "features" was too large to store in DynamoDB and was dropped
+```
+
+If caching is enabled in your configuration, the flag or segment may still be available in the SDK from the in-memory cache, but do not rely on this. If you see this message, consider redesigning your flag/segment configurations, or else do not use DynamoDB for the environment that contains this data item.
+
+This limitation does not apply to target lists in [Big Segments](https://docs.launchdarkly.com/home/users/big-segments/).
+
+A future version of the LaunchDarkly DynamoDB integration may use different strategies to work around this limitation, such as compressing the data or dividing it into multiple items. However, this integration is required to be interoperable with the DynamoDB integrations used by all the other LaunchDarkly SDKs and by the Relay Proxy, so any such change will only be made as part of a larger cross-platform release.
+
 ## About LaunchDarkly
 
 * LaunchDarkly is a continuous delivery platform that provides feature flags as a service and allows developers to iterate quickly and safely. We allow you to easily flag your features and manage them from the LaunchDarkly dashboard.  With LaunchDarkly, you can:

src/main/java/com/launchdarkly/sdk/server/integrations/DynamoDbDataStoreImpl.java

@@ -65,6 +65,11 @@ final class DynamoDbDataStoreImpl extends DynamoDbStoreImplBase implements Persi
   private static final String itemJsonAttribute = "item";
   private static final String deletedItemPlaceholder = "null"; // DynamoDB doesn't allow empty strings
 
+  // We won't try to store items whose total size exceeds this. The DynamoDB documentation says
+  // only "400KB", which probably means 400*1024, but to avoid any chance of trying to store a
+  // too-large item we are rounding it down.
+  private static final int DYNAMO_DB_MAX_ITEM_SIZE = 400000;
+
   private Runnable updateHook;
 
   DynamoDbDataStoreImpl(DynamoDbClient client, boolean wasExistingClient, String tableName, String prefix) {
@@ -108,6 +113,11 @@ public void init(FullDataSet<SerializedItemDescriptor> allData) {
       for (Map.Entry<String, SerializedItemDescriptor> itemEntry: entry.getValue().getItems()) {
         String key = itemEntry.getKey();
         Map<String, AttributeValue> encodedItem = marshalItem(kind, key, itemEntry.getValue());
+        
+        if (!checkSizeLimit(encodedItem)) {
+          continue;
+        }
+        
         requests.add(WriteRequest.builder().putRequest(builder -> builder.item(encodedItem)).build());
 
         Map.Entry<String, String> combinedKey = new AbstractMap.SimpleEntry<>(namespaceForKind(kind), key);
@@ -140,6 +150,9 @@ public void init(FullDataSet<SerializedItemDescriptor> allData) {
   @Override
   public boolean upsert(DataKind kind, String key, SerializedItemDescriptor newItem) {
     Map<String, AttributeValue> encodedItem = marshalItem(kind, key, newItem);
+    if (!checkSizeLimit(encodedItem)) {
+      return false;
+    }
 
     if (updateHook != null) { // instrumentation for tests
       updateHook.run();
@@ -271,4 +284,47 @@ static void batchWriteRequests(DynamoDbClient client, String tableName, List<Wri
       client.batchWriteItem(builder -> builder.requestItems(batchMap));
     }
   }
+  
+  private static boolean checkSizeLimit(Map<String, AttributeValue> item) {
+    // see: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/CapacityUnitCalculations.html
+    int size = 100; // fixed overhead for index data
+    for (Map.Entry<String, AttributeValue> kv: item.entrySet()) {
+      size += utf8Length(kv.getKey());
+      if (kv.getValue().s() != null) {
+        size += utf8Length(kv.getValue().s());
+      } else if (kv.getValue().n() != null) {
+        size += utf8Length(kv.getValue().n());
+      }
+    }
+    if (size <= DYNAMO_DB_MAX_ITEM_SIZE) {
+      return true;
+    }
+    logger.error("The item \"{}\" in \"{}\" was too large to store in DynamoDB and was dropped",
+        item.get(SORT_KEY).s(), item.get(PARTITION_KEY).s());
+    return false;
+  }
+  
+  private static int utf8Length(String s) {
+    // Unfortunately Java (at least Java 8) doesn't have a built-in way to determine the UTF8 encoding
+    // length without actually creating a new byte array, which we would rather not do. Guava does
+    // support this, but we don't want a dependency on Guava (except in test code).
+    if (s == null) {
+      return 0;
+    }
+    int count = 0;
+    for (int i = 0, len = s.length(); i < len; i++) {
+      char ch = s.charAt(i);
+      if (ch <= 0x7F) {
+        count++;
+      } else if (ch <= 0x7FF) {
+        count += 2;
+      } else if (Character.isHighSurrogate(ch)) {
+        count += 4;
+        ++i;
+      } else {
+        count += 3;
+      }
+    }
+    return count;
+  }
 }

src/test/java/com/launchdarkly/sdk/server/integrations/DynamoDbDataStoreImplTest.java

@@ -1,10 +1,29 @@
 package com.launchdarkly.sdk.server.integrations;
 
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.launchdarkly.sdk.server.DataModel;
+import com.launchdarkly.sdk.server.interfaces.DataStoreTypes.DataKind;
+import com.launchdarkly.sdk.server.interfaces.DataStoreTypes.FullDataSet;
+import com.launchdarkly.sdk.server.interfaces.DataStoreTypes.KeyedItems;
+import com.launchdarkly.sdk.server.interfaces.DataStoreTypes.SerializedItemDescriptor;
+import com.launchdarkly.sdk.server.interfaces.PersistentDataStore;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.AbstractMap.SimpleEntry;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
 import static com.launchdarkly.sdk.server.integrations.TestUtils.baseBuilder;
 import static com.launchdarkly.sdk.server.integrations.TestUtils.clearEverything;
 import static com.launchdarkly.sdk.server.integrations.TestUtils.createTableIfNecessary;
-
-import org.junit.BeforeClass;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
 
 /**
  * Runs the standard database data store test suite that's defined in the Java SDK.
@@ -17,7 +36,8 @@
  */
 @SuppressWarnings("javadoc")
 public class DynamoDbDataStoreImplTest extends PersistentDataStoreTestBase<DynamoDbDataStoreImpl> {
-
+  private static final String BAD_ITEM_KEY = "baditem";
+  
   @BeforeClass
   public static void setUpAll() {
     createTableIfNecessary();
@@ -43,4 +63,140 @@ protected boolean setUpdateHook(DynamoDbDataStoreImpl storeUnderTest, final Runn
     storeUnderTest.setUpdateHook(hook);
     return true;
   }
+
+  @Test
+  public void dataStoreSkipsAndLogsTooLargeItemOnInitForFlag() throws Exception {
+    dataStoreSkipsAndLogsTooLargeItemOnInit(DataModel.FEATURES, 0);
+  }
+
+  @Test
+  public void dataStoreSkipsAndLogsTooLargeItemOnInitForSegment() throws Exception {
+    dataStoreSkipsAndLogsTooLargeItemOnInit(DataModel.SEGMENTS, 1);
+  }
+
+  @Test
+  public void dataStoreSkipsAndLogsTooLargeItemOnUpsertForFlag() throws Exception {
+    dataStoreSkipsAndLogsTooLargeItemOnUpsert(DataModel.FEATURES);
+  }
+
+  @Test
+  public void dataStoreSkipsAndLogsTooLargeItemOnUpsertForSegment() throws Exception {
+    dataStoreSkipsAndLogsTooLargeItemOnUpsert(DataModel.SEGMENTS);
+  }
+
+  private void dataStoreSkipsAndLogsTooLargeItemOnInit(
+      DataKind dataKind,
+      int collIndex
+      ) throws Exception {
+    SerializedItemDescriptor badItem = makeTooBigItem(dataKind);
+    
+    // Construct a data set that is based on the hard-coded data from makeGoodData(), but with one
+    // oversized item inserted in either the flags collection or the segments collection.
+    FullDataSet<SerializedItemDescriptor> goodData = makeGoodData();
+    List<Map.Entry<DataKind, KeyedItems<SerializedItemDescriptor>>> dataPlusBadItem =
+        Lists.newArrayList(goodData.getData()); // converting the data set to a mutable list
+    List<Map.Entry<String, SerializedItemDescriptor>> items = Lists.newArrayList(
+        dataPlusBadItem.get(collIndex).getValue().getItems()); // this is now either list of flags or list of segments
+    items.add(0, new SimpleEntry<>(BAD_ITEM_KEY, badItem));
+    // putting the bad item first to prove that items after that one are still stored
+    dataPlusBadItem.set(collIndex, new SimpleEntry<>(dataKind, new KeyedItems<>(items)));
+    
+    // Initialize the store with this data set. It should not throw an exception, but instead just
+    // log an error and store all the *other* items-- so the resulting state should be the same as
+    // makeGoodData().
+    try (PersistentDataStore store = makeStore()) {
+      store.init(new FullDataSet<>(dataPlusBadItem));
+
+      assertDataSetsEqual(goodData, getAllData(store));
+    }
+  }
+  
+  private void dataStoreSkipsAndLogsTooLargeItemOnUpsert(
+      DataKind dataKind
+      ) throws Exception {
+    FullDataSet<SerializedItemDescriptor> goodData = makeGoodData();
+    
+    // Initialize the store with valid data. 
+    try (PersistentDataStore store = makeStore()) {
+      store.init(goodData);
+      
+      assertDataSetsEqual(goodData, getAllData(store));
+      
+      // Now try to store an oversized item. It should not throw an exception, but should not do
+      // the update-- so the resulting state should be the same valid data as before.
+      store.upsert(dataKind, BAD_ITEM_KEY, makeTooBigItem(dataKind));
+
+      assertDataSetsEqual(goodData, getAllData(store));
+    }
+  }
+  
+  private static FullDataSet<SerializedItemDescriptor> makeGoodData() {
+    return new FullDataSet<SerializedItemDescriptor>(ImmutableList.of(
+         new SimpleEntry<>(DataModel.FEATURES,
+             new KeyedItems<SerializedItemDescriptor>(ImmutableList.of(
+                 new SimpleEntry<>("flag1",
+                     new SerializedItemDescriptor(1, false, "{\"key\": \"flag1\", \"version\": 1}")),
+                 new SimpleEntry<>("flag2",
+                     new SerializedItemDescriptor(1, false, "{\"key\": \"flag2\", \"version\": 1}"))
+                 ))),
+         new SimpleEntry<>(DataModel.SEGMENTS,
+             new KeyedItems<SerializedItemDescriptor>(ImmutableList.of(
+                 new SimpleEntry<>("segment1",
+                     new SerializedItemDescriptor(1, false, "{\"key\": \"segment1\", \"version\": 1}")),
+                 new SimpleEntry<>("segment2",
+                     new SerializedItemDescriptor(1, false, "{\"key\": \"segment2\", \"version\": 1}"))
+                 )))
+         ));
+  }
+
+  private static SerializedItemDescriptor makeTooBigItem(DataKind dataKind) {
+    StringBuilder tooBigKeysListJson = new StringBuilder().append('[');
+    for (int i = 0; i < 40000; i++) {
+      if (i != 0 ) {
+        tooBigKeysListJson.append(',');
+      }
+      tooBigKeysListJson.append("\"key").append(i).append('"');
+    }
+    tooBigKeysListJson.append(']');
+    assertThat(tooBigKeysListJson.length(), greaterThan(400 * 1024));
+
+    String serializedItem;
+    if (dataKind == DataModel.SEGMENTS) {
+      serializedItem = "{\":key\":\"" + BAD_ITEM_KEY + "\", \"version\": 1, \"included\": " +
+          tooBigKeysListJson.toString() + "}";
+    } else {
+      serializedItem = "{\":key\":\"" + BAD_ITEM_KEY + "\", \"version\": 1, \"targets\": [{\"variation\": 0, \"values\":" +
+        tooBigKeysListJson.toString() + "}]}";
+    }
+    return new SerializedItemDescriptor(1, false, serializedItem);
+  }
+  
+  private static FullDataSet<SerializedItemDescriptor> getAllData(PersistentDataStore store) {
+    List<Map.Entry<DataKind, KeyedItems<SerializedItemDescriptor>>> colls = new ArrayList<>();
+    for (DataKind kind: new DataKind[] { DataModel.FEATURES, DataModel.SEGMENTS }) {
+      KeyedItems<SerializedItemDescriptor> items = store.getAll(kind);
+      colls.add(new SimpleEntry<>(kind, items));
+    }
+    return new FullDataSet<>(colls);
+  }
+  
+  private static void assertDataSetsEqual(FullDataSet<SerializedItemDescriptor> expected,
+      FullDataSet<SerializedItemDescriptor> actual) {
+    if (!actual.equals(expected)) {
+      throw new AssertionError("expected " + describeDataSet(expected) + " but got " +
+          describeDataSet(actual));
+    }
+  }
+
+  private static String describeDataSet(FullDataSet<SerializedItemDescriptor> data) {
+    return Joiner.on(", ").join(
+        Iterables.transform(data.getData(), entry -> {
+          DataKind kind = entry.getKey();
+          return "{" + kind + ": [" +
+            Joiner.on(", ").join(
+                Iterables.transform(entry.getValue().getItems(), item -> item.getValue().getSerializedItem())
+                ) +
+              "]}";
+        }));
+  }
 }