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

Expose NDC doc #2915

Merged
merged 10 commits into from
Dec 19, 2019
Merged

Expose NDC doc #2915

merged 10 commits into from
Dec 19, 2019

Conversation

wxing1292
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Dec 17, 2019

Coverage Status

Coverage increased (+0.02%) to 68.072% when pulling 6afc024 on ndc-design into 618d7cc on master.

docs/design/2290-cadence-ndc.md Outdated Show resolved Hide resolved
docs/design/2290-cadence-ndc.md Outdated Show resolved Hide resolved
docs/design/2290-cadence-ndc.md Outdated Show resolved Hide resolved
docs/design/2290-cadence-ndc.md Show resolved Hide resolved
docs/design/2290-cadence-ndc.md Outdated Show resolved Hide resolved
Copy link
Contributor

@meiliang86 meiliang86 left a comment

Choose a reason for hiding this comment

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

Thanks for writing this. Looks good overall. Some small comments.

1. version in the domain belongs to this data center, i.e.
`(vesion in domain) % (shared version increment) == (this data centers' initial version)`
2. the version of this workflow's last event is not less then version in domain, i.e.
`(last event's version) <= (version in domain)`
Copy link
Contributor

Choose a reason for hiding this comment

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

>=

Copy link
Contributor

Choose a reason for hiding this comment

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

if 1 is satisfied, what is the case that can lead to last event's version > version in domain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all replication tasks are handled asynchronously, meaning that domain replication (change of version) can experience delay.

| -------- | ------------- | --------------- | ------- |
```

T = 3: domain failover triggered, domain version is now 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it matter whether this is active or passive? Or are they the same (eventually)?
Maybe worth calling out explicitly.

docs/design/2290-cadence-ndc.md Show resolved Hide resolved

Since run 2 appears in data center B first, run 1 cannot be replicated as runnable due to rule `at most one run open` (see above), thus, `zombie` workflow state is introduced. Zombie state indicates a workflow which cannot be actively mutated by a data center (assuming corresponding domain is active in this data center). A zombie workflow can only be changed by replication task.

Run 1 will be replicated similar as run 2, except run 1's workflow state will be zombie before run 1 reaches completion.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, where do we keep this state? In DB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all persisted to database.

a workflow can potentially turn from a zombie workflow to current running workflow (in this data center), if say there is a data center which only has run 1 (no run 2) & the run 1 is running with a higher version.

@wxing1292 wxing1292 merged commit bd1f1d2 into master Dec 19, 2019
@wxing1292 wxing1292 deleted the ndc-design branch December 19, 2019 21:58
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.

5 participants