Alternator Streams does does not properly distinguish the type of change event

[nyh]

https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_streams_Record.html#DDB-Type-streams_Record-eventName explains that DynamoDB assigns to each change record one of three types:

1. INSERT - a new item was added to the table.
2. MODIFY - one or more of an existing item's attributes were modified.
3. REMOVE - the item was deleted from the table

This description has some interesting implications:

• Both PutItem or UpdateItem can be reported as either MODIFY or INSERT, depending on whether the item previously existed.
• Moreover, in both PutItem and UpdateItem, if the pre-existing item is identical to the new item, no change event is produced at all!
• DeleteItem of a non-existant item doesn't produce any change event at all.

Alternator does not currently implement any of these finer distinctions. All updates will be marked MODIFY, all deletes (whether or not an item existed) marked as REMOVE. This is a known issue (there is a comment about it in executor::get_records).

xfailing test/alternator test cases:

• test_streams.py::test_streams_updateitem_keys_only
• test_streams.py::test_streams_1_keys_only
• test_streams.py::test_streams_1_new_image
• test_streams.py::test_streams_1_old_image
• test_streams.py::test_streams_1_new_and_old_images

Implementing these distinctions in CDC will require it to do a read-before-write. But there are two serious problems to consider:

1. We also need these distinctions in the KEYS_ONLY case, where CDC doesn't do a read-before-write.
2. Alternator already does it's own read-before-write, and does it more correctly (with LWT). How can we get CDC to use that read for these decisions (as well as the pre/post-image), and not do its own read which might be less consistent?

CC @elcallio

[elcallio]

I think the key is to reduce the splitting of the alternator write into several CDC log rows. CDC can distinguish insert and update, so if we can ensure alternator uses update semantics when the data exists, and insert when not (i.e. row marker/not row marker), and fix the cdc log splitting, things should solve themselves. Big ifs...

[nyh]

@elcallio this is a nice idea, and can actually work. Alternator update operations could read the old value, decide if not to do the write at all (e.g., updating a value identical to the existing value, or deleting an already non-existent item), or if it's a new item (in which case write also the row marker) or there was a pre-existing value (in which case we won't write the row marker). The last case becomes more complex when replacing an existing item with PutItem (we can't use a row tombstone because that would remove the row marker as well) but still doable (we can delete all the individual columns instead of a row tombstone).

However, this idea has one performance drawback: It requires Alternator to always do a read-before-write - in Alternator - when CDC is enabled for that table. Although Alternator by default uses LWT for all writes, it doesn't actually do the read operation if the update requires it (e.g., it's a conditional update), and now, when CDC is enabled, it will need to do those extra read for all writes. And this is in addition to the read-before-write that CDC does (in a less consistent manner...) for OLD/NEW_IMAGE, when those are enabled. Eliminating this double read is my point 2 in the original issue description. We will still need to do more work than before in the KEYS_ONLY case, but maybe this extra work is worth for getting correctness (and CDC adds extra work in any case...).

[elcallio]

Or maybe just accept that we might give the wrong eventName if the alternator write is not read-before-write? If streams preimage is enabled we can further add the amount of correctness in reporting by simply keep some state tracking the the get_records code.
The result is that a KEYS_ONLY stream for an alternator table without RbW will have wrong eventNames, but other will be more correct.
Should probably find some hard info on how important the eventName distinctions are for end users.

[slivne]

pushing this out till we get some feedback