Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional updates? #30

Closed
mikecann opened this issue Jul 23, 2018 · 11 comments · Fixed by #47
Closed

Conditional updates? #30

mikecann opened this issue Jul 23, 2018 · 11 comments · Fixed by #47

Comments

@mikecann
Copy link

Hey, I cant see it anywhere but does this lib support conditional updates?

@breath103
Copy link
Contributor

Currently not! but as you can imagine, you can do
A)

const a = Model.primaryKey.get(10)
if (a.valueA > a.valueB) {
   a.valueA = a.valueB;
   await a.save()
}

instead of

B)

Model.writer.update({ hash: 10, set: ":valueA = :valueB", condition: ":valueA > valueB" })

Now,

  1. Only difference between those two method is 1 extra read. consistency, write capacity.. those are all pretty same. (Consistency might surprise you, but yes possbility of consistency issue is pretty much same here)

  2. I do think there are cases that you need solution (B), like when you have millions of records to update so you want to avoid using read capacity.

  3. But (B) does have a lot of issue to implement, such as
    a. field name mapping. what happens "valueA" mapped into value_a on ORM?
    b. no typings if you use "string" condition. if we do introduce typing, we've got to build our own condition builder something like

import { Conditions, Operators } from "dynamo-typeorm"
Model.writer.update({
  hash: 10,
  set: {
     valueA: valueB, // :valueA = :valueB
     valueA: Operators<Model>.add(valueB, Operator.const(10)), // :valueA = :valueB
  },
  // :valueA > :valueB AND (:valueA + 10) > :valueB
  condition: Conditions<Model>.And(
     Conditions<Model>.Gt("valueA", "valueB"),
     Conditions<Model>.Gt("valueA", Operators.add("valueA", Operators.const(10))),
  )
})

which is a "HUGE" work. https://github.com/typeorm/typeorm does pretty similar thing, but it is truly enormous amount of code to support all that operators and condition

The core value I really want to keep on this library is safely typed easy-to-use, fully typed typescript ORM for DynamoDB. if there is a way to keep that and pursue this feature, I'm more than happy to do that. but unfortunately, it seems like pretty hard for now.

Small trick my team use on this kinda matter, mostly for massive migration script,
we just use aws-sdk javascript library APIs :) We really belive that's better than introducing cripply typed APIs on framework.

@mikecann
Copy link
Author

Hey,

Thanks for your excellent reply! Really helpful 😄

Im not sure I follow your consistency argument however as I understand it the purpose of conditional writes / updates is to perform atomic operations for the use of transactions (https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/WorkingWithItems.html#WorkingWithItems.ConditionalUpdate) this is the main purpose we are requiring it for.

As far as I understand it, in your example A) that operation is handling on your own server not on the DynamoDB service thus it isnt atomic? Thus its no good for transactions.

If I understand it correctly your example B) would solve it (if it operates on the DynamoDB server) but as you say, it breaks type safefy which is really what you want to avoid.

Another library (https://github.com/lucasmafra/type-dynamo) handles it in an interesting way:

import { UserRepo } from './User'

async function saveUser() {
  const result = await UserRepo
                      .save(User)
                      .withCondition(attributeNotExists('id'))
                      .execute() 
  result.data.map(user => {
    console.log(user.id, user.name, user.email, user.age)
  })
}

Thoughts?

@breath103
Copy link
Contributor

breath103 commented Jul 24, 2018

Hm I mean it is "Atomic" in a sense that "write" won't happen if the given record is failed to meet given condition at that point
but it's very different from saying it's "Transaction" safe. As a matter of fact, in DynamoDB, there is no such thing as "transaction" at all, I mean literally. There isn't any way to group or rollback operations or anything close to that.

Thus the possible outcomes of conditional write are
a) Fail (because someone changed record before I save)
b) Success (nobody changed record before I save)

There is no case like "Lock the record while I'm saving this and next person can change it"

From the AWS doc example,

class User {
  id: number; // Primary Key
  name: string; 
  f1: string;
  f2: string;
  ...
}
  1. there is record { id: 100, name: "AA" }
  2. user A think its { id: 100, name: "AA" } tries to update it to { id: 100, name: "BB" } (at +0ms), this would take 10 ms
  3. user B think its { id: 100, name: "AA" } tries to update it to { id: 100, name: "CC" } (at +1ms), this would take 10 ms

In this example, if you're looking for an ability to make user B "fails" conditional update is what you looking for. but I'm just wondering,

A) in this case, is it really what you want to user B to "Fail"?
B) what happens user B is trying to update "name" only, but user A changed "f1 / f2" only? that's fine or do you need to put every single attributes of User record to Condition?
C) why don't you use really transaction safe DBs? you can literally "Lock" or "Limit access" on given record for those DBs, rather than just making it fails.

If you have real-life example for this application, please let me know! it seems like this kinda makes sense on really specific use cases? but I'm not sure that's even kind of cases that should be done with DynamoDB

If it seems like pretty common and meaningful for a lot of cases without requiring complicated typing, i'd surely support this.

For example, implementation like

User.writer.safeUpdate(oldRecord, newRecord)

(All attributes of OldRecord will be used for Condtition)

wouldn't break anything and pretty staightforward to implement.

@mikecann
Copy link
Author

Again thanks for your awesome reply.

In this example, if you're looking for an ability to make user B "fails" conditional update is what you looking for.

Yes this is what im looking for.

B) what happens user B is trying to update "name" only, but user A changed "f1 / f2" only? that's fine or do you need to put every single attributes of User record to Condition?

Well basically we need it to "fake" transactions by using a "lastUpdatedAt" field on the row. So we first query to return the document then submit the update with the condition that the "lastUpdatedAt" must not have changed. Otherwise we can backout of whatever transaction we were doing.

C) why don't you use really transaction safe DBs?

We are using serverless which imposes some rather severe restrictions on DB choices, so Dynamo is probably our only choice right now.

@breath103
Copy link
Contributor

👌 that could be meaningful use case l guess. i'd consider supporting

u = User.primaryKey.get(id)
const updatedAt = u.updatedAt;
u.updatedAt = Date.now();
u.name = "AA";

try {
   await User.writer.save(u, { condition: { updatedAt }})
} catch (e) {
   if (e instanceOf ConditionError) {
     console.log("failed to meet save condition")
   }
}

having one key (updatedAt / createdAt ..etc) for avoiding overriding, but that only :)

@mikecann
Copy link
Author

Cool :)

Tho I think maybe you might want to support a more general case:

u = User.primaryKey.get(id)
const updatedAt = u.updatedAt;
u.updatedAt = Date.now();
u.name = "AA";

try {
   await User.writer.save(u, { condition: { op: "!=", key: "updatedAt" }})
} catch (e) {
   if (e instanceOf ConditionError) {
     console.log("failed to meet save condition")
   }
}

Or something like that, im not massively familiar with how DynamoDB conditional writes work yet. Type-dynamo has an interesting functional way of doing it that still keeps it typesafe by using [keys of]

dynamoose handles it in a non-type-safe way (https://dynamoosejs.com/api#model):

odie.save({
    condition: '#o = :ownerId',
    conditionNames: { o: 'ownerId' },
    conditionValues: { ownerId: 4 }
  }, function (err) {
  if(err) { return console.log(err); }
  console.log('Ta-da!');
});

@breath103
Copy link
Contributor

breath103 commented Jul 24, 2018

Yeah checking attribute names are valid or not is totally possible. that could be handled by keysof (not perfectly but mostly, since it's would let user to put "virtual" attributes like get a() {} as a key also)

it's that currently there is no way to check typings for that variable
even if a: string, there is no way to not allow where({ a: ["eq", 100] })(But where({ a: 100 }) can be typed, since typescript support Partial typing such as partial User)

but anyway. as long as this not goes crazy flexible complicated condition, I'd do consider implementing this. But as a start, i'd love to focus on ["lastUpdatedAt", "==", 100] / ["lastUpdatedAt", "!==", 100] only.

And if it needs to be more complicated than that, as I mentioned earlier, I strongly recommend to not use DynamoDB. Cloud-based freely scalable RDS like Aurora is really best to fit those kinda usage

I've been migrating the whole system to serverless for about 1.5 years, and the big lesson was that RDS + Serverless could work totally fine also :) We did make mistakes of trying to put everything on DynamoDB also, which I can assure, doesn't work for "everything"

ANYWAY. I'll keep updating this issue

@mooyoul
Copy link
Contributor

mooyoul commented Nov 17, 2019

Hello there, I'm actively working on this feature.

Anyway, I'd like to share the reason why i decided to implement this functionality:

First, Orphaned/Incomplete item. updateItem API operation creates a new item with updated attributes if given item was not found. For example, Updating likes count right after card removal will create a new item with likes_count only.
Second, Overwriting. We have been using timestamp as range key. but new service has write-heavy scenario. it cannot guarantee sufficient uniqueness anymore even if timestamp has milliseconds precision.

These issues can be resolved by using attribute_exists(key) condition.

Current design is similar to TypeORM and most of operators will be supported even we don't need other operators.

For example:

// UpdateItem calls
await Card.primaryKey.update(12345, { likesCount: ["ADD", 1] }, { 
  condition: { id: attributeExists() } ,
});

// PutItem calls
const card = new Card();
await card.save({ condition: { id: attributeNotExists() } });

// PutItem calls
await card = new Card();
await Card.writer.put(card, { 
  condition: { id: attributeNotExists() },
});

// Complex conditions
await card.save({
  // same as: (status = 0 AND likesCount > 0) OR (userId IN (1,2,3,4))
  conditions: [
     { status: Equal(CardStatus.ACTIVE), likesCount: GreaterThan(0) },
     { userId: In([1,2, 3, 4]) },
  ],
});

@mooyoul mooyoul mentioned this issue Nov 17, 2019
5 tasks
@benhutchins
Copy link

I forked dynamo-types and made a lot of changes, my fork called Dyngoose supports conditional writes and is somewhat compatible with dynamo-types.

https://github.com/benhutchins/dyngoose

The work in #47 looks like some great work too @mooyoul

@breath103
Copy link
Contributor

🎊 this has been added to v2.9.0. for usage, checkout readme.md or .update or .delete methods.
@mikecann @mooyoul

@breath103
Copy link
Contributor

@mikecann as of transaction - https://aws.amazon.com/blogs/aws/new-amazon-dynamodb-transactions/ it's actually being supported as native API. will work on to support this soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants