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

[docs] add dynamodb schema document #9

Merged
merged 4 commits into from
Mar 3, 2022
Merged

Conversation

harunalfat
Copy link
Collaborator

Summary:

  • Add document for dynamoDB schema

@harunalfat harunalfat self-assigned this Mar 2, 2022
docs/dynamodb.md Outdated
Comment on lines 16 to 17
- `can_be_partner`, Boolean => flag for a pokemon that be able to be choose as a partner
- `can_be_enemy`, Boolean => flag for a pokemon that be able to become an enemy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these fields should be replaced with following field:

type, String => the pokemon type, valid values: PARTNER, ENEMY

Then we could use this field as partition key in global secondary index (GSI).

So when we want to query for available Pokemon partners, we could query this GSI with type=PARTNER or type=ENEMY if we want to query enemies.

If we are still using can_be_partner & can_be_enemy fields, we need to scan the whole table first then apply the filter. We should avoid scan operation in DynamoDB since it is usually very expensive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make a Pokemon support both of the types?

e.g. Pikachu can be either partner or enemy. Or this is a case that we do not need to support currently?

Maybe we can put the roles (PARTNER, ENEMY) in a set and make it indexable? I'm totally newbie with DynamoDB here.. 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can. You can make the type field optional. So in Pikachu we could set type=PARTNER.

When we want to query partner, we could use the type index. If we want to query enemy we could use scan table operation.

Maybe we can put the roles (PARTNER, ENEMY) in a set and make it indexable?

No, in DynamoDB this is not possible. Index in DynamoDB only support scalar types.

I'm totally newbie with DynamoDB here.. 😅

No problem, this common pitfall for someone who just use DynamoDB. 😃

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaah, understood, that's how the index query works. Okay, lemme update necessary things

docs/dynamodb.md Outdated Show resolved Hide resolved
docs/dynamodb.md Outdated
Comment on lines 81 to 85
- `partner.health`, Number => remaining health of player's partner
- `enemy.id`, String => identifier of opposite partner
- `enemy.health`, Number => remaining health of opposite partner
- `last_damage.partner`, Number => last inflicted damage to player's partner
- `last_damage.enemy`, Number => last inflicted damage to opposite partner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the definition of last_damage, partner, and enemy field. It should be Map type

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it 👌

docs/dynamodb.md Outdated
Comment on lines 11 to 14
- `battle_stats.max_health`, Number => maximum health (on battle start) of a pokemon
- `battle_stats.attack`, Number => number of damage that can be inflicted by a pokemon
- `battle_stats.defense`, Number => number of damage reducer for a pokemon (damage = enemy.attack - your_partner.defense)
- `battle_stats.speed`, Number => chance for getting a turn in battle, higher means more likely to get a turn in battle RNG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the definition of battle_stats field. It should be Map type

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted 👌

@harunalfat harunalfat requested a review from riandyrn March 3, 2022 04:15
docs/dynamodb.md Outdated
Comment on lines 12 to 15
- `battle_stats.max_health`, Number => maximum health (on battle start) of a pokemon
- `battle_stats.attack`, Number => number of damage that can be inflicted by a pokemon
- `battle_stats.defense`, Number => number of damage reducer for a pokemon (damage = enemy.attack - your_partner.defense)
- `battle_stats.speed`, Number => chance for getting a turn in battle, higher means more likely to get a turn in battle RNG
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better if you add 1 indentation here. This is to make it clear max_health, attack, defense, & speed is under battle_stats. In think it is okay too if you remove the battle_stats prefix.

Here is what I mean:

  • battle_stats, Map => holds battle stats related information of a pokemon
    • max_health, Number => maximum health (on battle start) of a pokemon
    • attack, Number => number of damage that can be inflicted by a pokemon
    • defense, Number => number of damage reducer for a pokemon (damage = enemy.attack - your_partner.defense)
    • speed, Number => chance for getting a turn in battle, higher means more likely to get a turn in battle RNG

docs/dynamodb.md Outdated
- `battle_stats.max_health`, Number => maximum health (on battle start) of a pokemon
- `battle_stats.attack`, Number => number of damage that can be inflicted by a pokemon
- `battle_stats.defense`, Number => number of damage reducer for a pokemon (damage = enemy.attack - your_partner.defense)
- `battle_stats.speed`, Number => chance for getting a turn in battle, higher means more likely to get a turn in battle RNG
- `avatar_url`, String => url for avatar image of a pokemon
- `can_be_partner`, Boolean => flag for a pokemon that be able to be choose as a partner
- `can_be_enemy`, Boolean => flag for a pokemon that be able to become an enemy
- `type`, String => the pokemon type, valid values: `PARTNER`, `ENEMY`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you don't want to make it possible for Pikachu to also become possible enemy?

If do you want to do so, write the spec like this:

  • type, String, OPTIONAL => the pokemon type, valid values: PARTNER

You can also change the name type into extra_role (since this is optional), so the spec would be:

  • extra_role, String, OPTIONAL => the pokemon type, valid values: PARTNER

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I think extra_role is more fit than type, also to avoid using reserved words. Noted 👌

docs/dynamodb.md Outdated
},
```

**Relevant Indexes:**
- `PRIMARY_KEY` => `id`
- `GLOBAL_SECONDARY_INDEX` => `type`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be:

  • type => type

This is the style we usually use to define GSI. Or you could also make it like this:

  • type, GSI => type

This is to make it clear that this index is GSI not LSI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted 👌

docs/dynamodb.md Outdated
Comment on lines 81 to 85
- `partner`, Map => holds information of player's pokemon
- `partner.health`, Number => remaining health of player's partner
- `enemy`, Map => holds information of enemy's pokemon
- `enemy.id`, String => identifier of opposite partner
- `enemy.health`, Number => remaining health of opposite partner
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it like this:

  • partner, Map => holds information of player's pokemon
    • health, Number => remaining health of player's partner
  • enemy, Map => holds information of enemy's pokemon
    • id, String => identifier of opposite partner
    • health, Number => remaining health of opposite partner

@riandyrn
Copy link
Collaborator

riandyrn commented Mar 3, 2022

Btw, in Battles table, why don't you put all the data there, @harunalfat?

Like for example the complete data for partner & enemy.

By doing this when you are fetching the battle data you could only fetch it one time. No need to lookup from Pokemon Table.

This is also the same case with partner data in Games.

I think by putting all the data in the object creation it will make the query more efficient. Notice that DynamoDB is key value store similar to Redis. So you cannot have join queries.

@harunalfat
Copy link
Collaborator Author

harunalfat commented Mar 3, 2022

Btw, in Battles table, why don't you put all the data there, @harunalfat?

Like for example the complete data for partner & enemy.

By doing this when you are fetching the battle data you could only fetch it one time. No need to lookup from Pokemon Table.

This is also the same case with partner data in Games.

My main concern is data consistency, if a Pokemon get an update in its battle stats, should it be reflected in running games or not? This is a very common concern with NoSQL. If running battles won't reflect Pokemon stats update, I agree filling all necessary data altogether is a much better way

@riandyrn
Copy link
Collaborator

riandyrn commented Mar 3, 2022

My main concern is data consistency, if a Pokemon get an update in its battle stats, should it be reflected in running games or not?

Hmm.., ok, let say we do some adjustment to Pokemon data.

This will affect:

  • partner & enemy data on running battle
  • partner data on running game

From the context of running battle, I think it's better if we keep the battle data as before the adjustment. By doing this we could keep the consistency for that running battle.

For the running game perspective I agree with you.

@harunalfat
Copy link
Collaborator Author

harunalfat commented Mar 3, 2022

Hmm.., ok, let say we do some adjustment to Pokemon data.

This will affect:

  • partner & enemy data on running battle
  • partner data on running game

From the context of running battle, I think it's better if we keep the battle data as before the adjustment. By doing this we could keep the consistency for that running battle.

Ok, seems good. So, I'll persist states of both partner and enemy into Battle entity. It'll becomes

{
    "game_id": "1a34a63d-afe6-4186-8628-13a25eaa6076",
    "state": "DECIDE_TURN",
    "partner": {
          "id": "b1c87c5c-2ac3-471d-9880-4812552ee15d",
          "name": "Pikachu",
          "battle_stats": {
              "max_health": 100,
              "attack": 25,
              "defense": 5,
              "speed": 15,
              "health": 75
          },
          "avatar_url": "https://assets.pokemon.com/assets/cms2/img/pokedex/full/025.png",
          "extra_role": "PARTNER"
      },
    "enemy":{
        "id": "88a98dee-ce84-4afb-b5a8-7cc07535f73f",
        "name": "Squirtle",
        "battle_stats": {
            "max_health": 100,
            "attack": 20,
            "defense": 10,
            "speed": 15,
            "health": 60
        },
        "avatar_url": "https://assets.pokemon.com/assets/cms2/img/pokedex/full/007.png"
      },
    "last_damage": {
        "partner": 10,
        "enemy": 25
    }
}

and when running a new battle, all pokemons data will be re-pulled again from Pokemons table. Is this good enough?

And seems like partner data in game entity only acts as an argument for new battle's partner, which will be re-pulled. So, I don't think partner field in Games need to be filled with complete data. WDYT?

@riandyrn
Copy link
Collaborator

riandyrn commented Mar 3, 2022

when running a new battle, all pokemons data will be re-pulled again from Pokemons table. Is this good enough?

Yup👍🏻

So, I don't think partner field in Games need to be filled with complete data. WDYT?

Yup, you are right.

@harunalfat harunalfat requested a review from riandyrn March 3, 2022 09:16
Copy link
Collaborator

@riandyrn riandyrn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @harunalfat! 🎉🎉🎉

You may now build the adapter. 👍🏻

@riandyrn riandyrn merged commit 478c380 into master Mar 3, 2022
@riandyrn riandyrn deleted the docs/dynamodb-document branch March 3, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants