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

Introduce Dynamo model #69

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Introduce Dynamo model #69

merged 4 commits into from
Oct 10, 2023

Conversation

raj14243
Copy link

No description provided.

@raj14243 raj14243 requested a review from jolynch September 29, 2023 01:00
Copy link
Contributor

@jolynch jolynch left a comment

Choose a reason for hiding this comment

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

Awesome! Love the idea of the "managed" instance/drive to skip simulation.

Just a few pieces of feedback and some nitpicks.

service_capacity_modeling/interface.py Outdated Show resolved Hide resolved
service_capacity_modeling/models/common.py Outdated Show resolved Hide resolved
service_capacity_modeling/models/org/netflix/__init__.py Outdated Show resolved Hide resolved
service_capacity_modeling/models/org/netflix/ddb.py Outdated Show resolved Hide resolved
service_capacity_modeling/models/org/netflix/ddb.py Outdated Show resolved Hide resolved
AccessConsistency.best_effort,
AccessConsistency.eventual,
AccessConsistency.read_your_writes,
AccessConsistency.never,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe DDB can do up to serializable (essentially all consistency levels) in the same region

Copy link
Author

Choose a reason for hiding this comment

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

let me update this

Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably determine the default strong/weak via this as well (if someone asks for eventual, you don't need to bill for strong reads)

service_capacity_modeling/models/org/netflix/ddb.py Outdated Show resolved Hide resolved
service_capacity_modeling/models/org/netflix/ddb.py Outdated Show resolved Hide resolved
@@ -325,6 +325,92 @@ def models(self) -> Dict[str, CapacityModel]:
def hardware_shapes(self) -> HardwareShapes:
return self._shapes

def _plan_percentiles(
Copy link
Author

Choose a reason for hiding this comment

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

Introduced this method to address the following issue.

Previously the steps to generate percentile plan was

  • generate percentile desires
  • plan_certain using the generated desires
    Issue in the above is, when percentile desires are generated the attributes that are not supplied by user are replaced with defaults declared in CapacityDesires interface. This results in parent_desires.merge_with not merging the defaults i.e. not merging the model defaults for attributes that are not supplied by user.

To mitigate the above issue, we first generate desires by merging with model defaults and then generate the percentile desires using the merged desires.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find

@raj14243 raj14243 force-pushed the feature/rummadisetty/ddb-model branch from 3f2041e to 4fb594f Compare October 6, 2023 23:37
@raj14243 raj14243 force-pushed the feature/rummadisetty/ddb-model branch from 4fb594f to 0f4e682 Compare October 6, 2023 23:41
@jolynch jolynch merged commit 778c6aa into main Oct 10, 2023
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.

2 participants