Update our EDD process documentation #166

joseph-flinn · 2023-08-04T18:02:49Z

Objectives

Update our EDD documentation with the solution found to support EDD, meeting all the unique constraints between Cloud and Standard Self-host environments
Add a future type of migration that can be run manually as needed

bitwarden-bot · 2023-08-04T18:04:11Z

Checkmarx One – Scan Summary & Details – 9a17ed1a-de9c-4855-9430-6ebd568f6dde

No New Or Fixed Issues Found

cloudflare-workers-and-pages · 2023-08-04T18:05:41Z

Deploying with Cloudflare Pages

Latest commit:	`f420880`
Status:	✅ Deploy successful!
Preview URL:	https://69d1a224.contributing-docs.pages.dev
Branch Preview URL:	https://update-db-migrations-docs.contributing-docs.pages.dev

View logs

withinfocus

Few things to consider.

withinfocus · 2023-08-07T18:08:46Z

docs/contributing/database-migrations/edd_state_machine.jpg

💭 What id we sliced this up into thirds so it can be read top to bottom?

Took a stab at refiguring the image. Thoughts on the new format?

docs/contributing/database-migrations/edd.mdx

withinfocus · 2023-08-07T18:35:02Z

docs/contributing/database-migrations/edd.mdx

+There are some unique constraints to Bitwarden that are not addressed directly in Martin Fowler's
+EDD article.
+
+- Our Production instances in the cloud are required to be on at all times


⛏️ When possible I think it's a good idea to eliminate personal pronouns; there's a lot of "our" and "we" in this document.

I removed all of the personal pronouns that I added. I did not update the documented process that already existed.

withinfocus · 2023-08-07T18:36:27Z

docs/contributing/database-migrations/edd.mdx


-### Devops
+### Cloud Environments


💭 Is "cloud" the term we want? Or perhaps more generically "online"?

I went with "always-on" over "online" to avoid the ambiguous "online application" possibly referring to any application on the internet. Thoughts?

docs/contributing/database-migrations/edd.mdx

withinfocus · 2023-08-07T18:38:45Z

docs/contributing/database-migrations/index.md

@@ -72,4 +72,15 @@ pwsh ef_migrate.ps1 [NAME_OF_MIGRATION]

 This will generate the migrations, which should then be included in your PR.

+### [Not Yet Implemented] Manual MSSQL Migrations


❓ Will this effectively be implemented once we publish these documents though?

Also, there's a few references in this document to "future" that you're limiting on the EDD one.

I don't think so. We need to implement EDD as soon as possible because we are already using EDD without the processes to support it in the new EU environments and also in the self-host environments. I am considering this out of scope for the EDD process implementation and do not have a timeline on when we will get to this. I'm hoping that this note here will avoid the issue that we are running into now with needing to scramble to add in automated processes to support all of our environment types.

docs/contributing/database-migrations/edd.mdx

Hinton

Some initial thoughts.

docs/contributing/database-migrations/edd.mdx

Hinton · 2023-08-08T15:14:36Z

docs/contributing/database-migrations/edd.mdx

+There are some unique constraints to Bitwarden that are not addressed directly in Martin Fowler's
+EDD article.
+
+- Our Production instances in the cloud are required to be on at all times
+- Our self-host instances that we do not directly have access to manage must support the same EDD
+  processes; however, they do not have the same always-on application constraint
+- Minimization of manual steps in our process


These are in my opinion all considered in the article.

That's the purpose of EDD.

Self-hosted does not really need to support EDD, the current deployment mechanism turns off the server,r runs migrations and starts the new code instances. I.e. there is no transition phase.

I've updated the language a bit to focus more on the implementation of orchestrating EDD in our environment rather than EDD itself.

After reading through Fowler's article, I came away with the impression that the purpose of EDD is not to support an always-on DB/application. He went so far as to say that it would take a whole separate article to examine the technicalities to orchestrate EDD in such an environment. I think it is important to keep the always-on requirement for orchestration in mind.

I see this a different way. Exiting the Transition Phase is not done until the Finalization migrations have been run in the next update. If every release of server has destructive DB changes, I would say that the self-host deployment will constantly switch from one Transition Phase to the next.

Hinton · 2023-08-08T15:16:11Z

docs/contributing/database-migrations/edd.mdx

+We tweak the terminology to be more easily understandable in how EDD relates to our deployment
+processes in both our environments: our always-on application in the cloud and our self-host
+deployments. We use the terms: _Initial_ Phase (instead of _Start_), _Transition_ Phase, and
+_Finalization_ Phase (instead of _End_).


I'm always hesitant to diverge from established naming but I wonder if start here more accurately refer to the beginning of the development work, not deployment.

I agree. I have updated the terminology to better align with the diagram and not redefine the established naming. The added "types" of migrations is to help overlay EDD on the deployment process and to clearly communicate between Development and *Ops the expectations that come with those migrations.

docs/contributing/database-migrations/edd.mdx

Hinton · 2023-08-08T15:19:02Z

docs/contributing/database-migrations/edd.mdx

+  functionality
+- Supports both the previous version of code and the one being upgraded to
+- Run during upgrade
+- Must execute quickly to minimize downtime.


Define quickly. Do you mean non-locking, or few operations?

With k8s deployments, the goal is to get an end-to-end automated deploy finished within ten minutes. DB migrations are run in a serial manner. So any DB changes that put this initial target at risk would be considered "not quick enough". Any schema changes that put this target at risk should be flagged for more in depth discussion to see if it can be moved to during the Transition Phase.

Hinton · 2023-08-08T15:32:52Z

docs/contributing/database-migrations/edd.mdx

+#### Initial Phase
+
+- Compatible with <i>previous</i> <b>and</b> <i>next</i> application code changes
+- Represents the beginning of a database change
+- Updates our database schema to support any new functionality while also maintaining old
+  functionality
+- Supports both the previous version of code and the one being upgraded to
+- Run during upgrade
+- Must execute quickly to minimize downtime.
+
+#### Transition Phase
+
+- Compatible with <i>previous</i> <b>and</b> <i>next</i> application code changes
+- The time between initial migration and finalization
+- Exists to provide an opportunity to rollback server to _previous_ version prior to breaking
+  changes
+- Only data population migrations may be run at this time, if they are needed
+  - Optional step, required only when migrating data would be too slow to execute during the initial
+    migration. This might be a column population, index creation, anything to prepare our database
+    for the _next_ version
+  - Must be run as a background task during our Transition phase.
+  - These MUST run in a way where the database stays responsive during the full migration
+- Schema changes are NOT to be run during this phase.
+
+#### Finalization Phase
+
+- Only compatible with <i>next</i> application code; represents the point of no return for this
+  migration
+- Removes columns, data, and fallback code required to support _previous_ version
+- Should be run as a typical migration either during a subsequent upgrade


I'm not a fan of these lists, i don't believe they convey much information and they are hard to understand.

I wonder if it's better to look at the transitions instead of the phases. There are two database transitions and in between these transitions we could if we want have any number of code deployments.

Migrate to a database structure that supports old and new schema.

Migrate to a database structure that only supports new schema

Migrate data

Migrate structure

The problem occurs because Migrate data can be both load and time consuming, and should therefore be done at our convenience in order to minimize database load. It technically doesn't need to be executed until just before the next code deployment if that simplifies things.

Thanks for the feedback on the lists. What format would you suggest to use to clearly define each of these types of migrations? Clear understanding of them is important for developers putting new migrations in the correct places in our implementation of EDD in our deploy processes.

…re clear

Hinton

Still reviewing this, going to continue tomorrow but here are some comments. I also rebased #165 on top of this.

Hinton · 2023-08-10T12:07:15Z

docs/contributing/database-migrations/edd.mdx

+import refactoringPhases from "./stages_refactoring.jpg";
+import eddStateMachine from "./edd_state_machine.jpg";


Is there a reason for importing images this way?

Yes. To support JSX for this feedback: #166 (comment)

We don't need JXS though. It works just as well to define the image link in the html.

Any suggestions on the change? I tried updating the usage of the the JSX and it led to a broken link:

<img src='./stages_refactoring.jpg' alt="Refactoring Phases" />

docs/contributing/database-migrations/edd.mdx

Hinton · 2023-08-10T12:10:09Z

docs/contributing/database-migrations/edd.mdx

This document should be written with the aim to give a high level overview of how Evolutionary database design works.

Developer focused documentation on how to write migrations should be in either the MSSQL or EF files.

What portion of the article does not line up with the high level overview of EDD?

docs/contributing/database-migrations/edd.mdx

Thomas-Avery

Thank you for your work on this!

docs/contributing/database-migrations/edd.mdx

Co-authored-by: Thomas Avery <[email protected]>

docs/contributing/database-migrations/edd.mdx

Hinton · 2023-08-22T11:37:33Z

@joseph-flinn I rewrote portions of this in #184, let me know what you think.

eliykat

I'm just catching up on these changes and I noticed a typo below.

I read this version, then @Hinton's version in #184, and I found #184 to be a bit clearer in its explanations of the different phases. I'd support those tweaks being merged in here.

eliykat · 2023-08-23T04:17:08Z

docs/contributing/database-migrations/edd.mdx


-### Developer
+Schema migrations and data migrations as just migrations. The underlying implementation issue is


Suggested change

Schema migrations and data migrations as just migrations. The underlying implementation issue is

Schema migrations and data migrations are just migrations. The underlying implementation issue is

joseph-flinn · 2023-08-29T20:35:15Z

@joseph-flinn I rewrote portions of this in #184, let me know what you think.

@Hinton Thanks so much! I think they are all great rewrites. I left a couple of comments in conjunction with the ones from @withinfocus

Hinton · 2023-09-21T17:57:54Z

docs/contributing/database-migrations/edd_state_machine.jpg

@joseph-flinn could we add the source to this image if we need to modify it in the future?

Hinton

I haven't focused on the deployment orchestration since it's outside my area of expertise, but the rest of the document looks good to me.

withinfocus

These are a lot of changes over a long period of time so I think it's best to get this in and do another round once we finalize the actual changes DevOps is making.

…ps/migrations * 'master' of github.com:bitwarden/contributing-docs: (37 commits) chore(deps): update dependency cspell to v7.3.7 (#206) [PM-4161] Fix build command on README.md (#207) Fixed typo in csharp.md (#180) Update our EDD process documentation (#166) chore(deps): update actions/checkout action to v4.1.0 (#204) chore(deps): lock file maintenance (#205) fix(deps): update npm minor to v2.4.3 (#203) use dash when running self-hosted dotnet profile (#202) chore(deps): update actions/checkout action to v4 (#191) chore(deps): lock file maintenance (#201) chore(deps): update npm minor (#195) fix(deps): update dependency docusaurus-lunr-search to v3 (#200) setup_secrets: Pass `-clear` as switch (#194) Update KeyConnector docs for ARM Macs (#171) Lock file maintenance (#193) Update dependency cspell to v7.3.5 (#192) Update dependency ubuntu to v22 (#174) Update gh minor (#181) Feature flag documentation updates and mentions about new local override capabilities (#187) Update npm minor (#188) ... # Conflicts: # custom-words.txt # docs/contributing/database-migrations/edd.mdx # docs/contributing/database-migrations/index.md

Initial pass at updating our EDD database change processes

1ae24e1

joseph-flinn requested a review from a team as a code owner August 4, 2023 18:02

Fix file name of new image

a7db292

joseph-flinn marked this pull request as draft August 4, 2023 18:07

Switch from 'rerunnable' to 'repeatable'

e0cd9d8

withinfocus requested changes Aug 7, 2023

View reviewed changes

Push the quick fixes from feedback

9df35ad

joseph-flinn marked this pull request as ready for review August 8, 2023 14:36

Hinton reviewed Aug 8, 2023

View reviewed changes

docs/contributing/database-migrations/edd.mdx Outdated Show resolved Hide resolved

Hinton reviewed Aug 8, 2023

View reviewed changes

joseph-flinn added 7 commits August 9, 2023 13:14

Removed repeated use of Fowler's name as well as the repeated use of EDD

09e4213

Fix the image caption

4717bea

Removing all added personal pronouns

e00c9a8

Use markdown text styling

2a11473

Rename the application code version in the Phase definitions to be mo…

a37c363

…re clear

Update terminology definitions

29ff1b7

Update language to be more focused

bba1df6

Hinton reviewed Aug 10, 2023

View reviewed changes

Accepted introduction summary improvements

0b2da50

joseph-flinn requested a review from withinfocus August 10, 2023 19:05

joseph-flinn added 2 commits August 10, 2023 12:15

Accepted suggested changes.

cc1150e

Accept revised defenition and examples of non-destructive changes

4fedfc9

This was referenced Aug 15, 2023

Rename DbScripts_future and DbScripts_data_migrations bitwarden/server#3192

Merged

Rename DbScripts_future to DbScripts_finalization #177

Merged

Thomas-Avery reviewed Aug 16, 2023

View reviewed changes

docs/contributing/database-migrations/edd.mdx Outdated Show resolved Hide resolved

Update docs/contributing/database-migrations/edd.mdx

9ea294d

Co-authored-by: Thomas Avery <[email protected]>

Hinton reviewed Aug 17, 2023

View reviewed changes

docs/contributing/database-migrations/edd.mdx Outdated Show resolved Hide resolved

addtional updates

818cecf

joseph-flinn and others added 2 commits August 18, 2023 11:40

revert to JSX to fix the broken image link

602f9ca

Merge branch 'master' into update-db-migrations-docs

46b5f6f

eliykat reviewed Aug 23, 2023

View reviewed changes

joseph-flinn and others added 3 commits September 14, 2023 12:11

Merge branch 'master' into update-db-migrations-docs

46dcc12

Merge branch 'master' into update-db-migrations-docs

e4f5218

Tweak the EDD doc (#184)

f420880

Hinton approved these changes Sep 21, 2023

View reviewed changes

Hinton reviewed Sep 21, 2023

View reviewed changes

Hinton approved these changes Sep 21, 2023

View reviewed changes

withinfocus approved these changes Sep 21, 2023

View reviewed changes

joseph-flinn merged commit 9bd0a72 into master Sep 26, 2023
3 checks passed

joseph-flinn deleted the update-db-migrations-docs branch September 26, 2023 18:43

		@@ -72,4 +72,15 @@ pwsh ef_migrate.ps1 [NAME_OF_MIGRATION]

		This will generate the migrations, which should then be included in your PR.

		### [Not Yet Implemented] Manual MSSQL Migrations

		import refactoringPhases from "./stages_refactoring.jpg";
		import eddStateMachine from "./edd_state_machine.jpg";


		### Developer
		Schema migrations and data migrations as just migrations. The underlying implementation issue is

	Schema migrations and data migrations as just migrations. The underlying implementation issue is
	Schema migrations and data migrations are just migrations. The underlying implementation issue is

Update our EDD process documentation #166

Update our EDD process documentation #166

Conversation

joseph-flinn commented Aug 4, 2023

Objectives

bitwarden-bot commented Aug 4, 2023 • edited Loading

No New Or Fixed Issues Found

cloudflare-workers-and-pages bot commented Aug 4, 2023 • edited Loading

Deploying with Cloudflare Pages

withinfocus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hinton left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hinton left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thomas-Avery left a comment

Choose a reason for hiding this comment

Hinton commented Aug 22, 2023

eliykat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-flinn commented Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Hinton left a comment

Choose a reason for hiding this comment

withinfocus left a comment

Choose a reason for hiding this comment

bitwarden-bot commented Aug 4, 2023 •

edited

Loading

cloudflare-workers-and-pages bot commented Aug 4, 2023 •

edited

Loading

Hinton left a comment •

edited

Loading

Hinton left a comment •

edited

Loading

joseph-flinn commented Aug 29, 2023 •

edited

Loading