Skip to content

Commit

Permalink
Add ADRs to document changes to metadata tabels
Browse files Browse the repository at this point in the history
These ADRs document the canges conducted in:
- #3402
- #3400
- #3394
- #3401
- #3393
  • Loading branch information
FloThinksPi committed Aug 23, 2023
1 parent 773e3cf commit 218f103
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 0 deletions.
57 changes: 57 additions & 0 deletions decisions/0011-make-labels-and-annotations-unique.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
4: Adding key prefix to annotations
================================

Date: 2023-08-23

Status
------

Accepted

Context
-------

The Database for labels and annotation tables currently lacks unique constraints, which can lead to the potential insertion of duplicate metadata (labels/annotations) due to a race condition. This occurs when two CC VMs receive an update request concurrently, find no existing metadata object in the table, and subsequently, both issue a create. Given the absence of preventative measures in the DB schema, this results in duplicate entries (sometimes with differing values) in the Database, leading to undefined behavior.
The inconsistency in the metadata can cause the API to return different results for different calls. To ensure data integrity and consistency, it is crucial to prevent duplicates in the Database for metadata objects.
Our proposed solution is to set a unique constraint on labels(resource_id, key_prefix, key_name) and annotations(resource_id, key_prefix, key). However, there are several non-obvious considerations related to Postgres and MySQL behavior.

1. NULL values in the Database (both MySQL and Postgres) are always Unique as NULL!=NULL in SQL. To make the Uniqueness constraint effective, we must not allow NULL values.
1. The Key column in the Annotations Tables is of type 'varchar(1000)'. This length is too long for MySQL to create a unique constraint on it. As the API docs and the CC limit the key length for annotations to 63 characters, adjusting this column's length is feasible.
1. During the removal of duplicates from the metadata tables, it is critical to ensure no new duplicates are inserted while the DB migration runs. Otherwise, it would fail setting the unique constraint. We need to use table locking during a table's migration.

Decision
--------

1. We will modify the CC Code to insert an empty string into the Database but convert this to nil in the Model to maintain current behavior and not to have compare string values. The CC can handle both an empty string in the columns 'key_prefix' and 'key' columns and the NULL value at this point.
1. We will add code in the CC to manage unique constraint violations when updating labels/annotations and retry once to eliminate the race condition and ensure a select returns a valid object, thereby preventing a second create on a metadata table.
1. We will introduce a migration that modifies the schema and sanitizes the metadata tables:
1. For Annotations Tables:
1. Lock the Table
1. Trim Keys longer than 63 characters (this should never actually occur, but is done to prevent migration failure)
1. Reduce the Column Length of the Key column to 63 characters
1. Convert all NULL values in the 'key_prefix' column to an empty string
1. Set a NOT NULL constraint on columns (resource_id, key_prefix, key)
1. Find and delete all duplicate values where the columns (resource_id, key_prefix, key) are equal
1. Set a unique constraint for columns (resource_id, key_prefix, key)
1. For Labels Tables:
1. Lock the Table
1. Convert all NULL values in the 'key_prefix' column to an empty string
1. Set a NOT NULL constraint on columns (resource_id, key_prefix, key_name)
1. Find and delete all duplicate values where the columns (resource_id, key_prefix, key_name) are equal
1. Set a unique constraint for columns (resource_id, key_prefix, key_name)
1. We will modify the code to clean up and stop handling NULL values from the DB as the can just be empty strings in the DB at this point.

Please note:
1. Steps 1 and 2 are backward compatible with any CC version.
1. Steps 3 and 4 are backward compatible with any CC version that includes the changes from Step 1 and 2.

Consequences
------------

### Positive Consequences

1. The API will behave correctly as duplicates will no longer occur.

### Negative Consequences

1. A staged rollout will be necessary, requiring clear documentation about limitations on upgrade paths, as we have two changes that necessitate a specific minimum CF-Deployment/CAPI version.
39 changes: 39 additions & 0 deletions decisions/0012-migrating-key-to-key_name-for-annotations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
4: Adding key prefix to annotations
================================

Date: 2023-08-23

Status
------

Accepted

Context
-------

In our ADR-0004, the decision to align annotations with labels was made, while keeping in mind that no data migration should take place. However, following ADR-11, ensuring unique annotations mandated a data migration. This presented an opportunity to align the database layout for Annotations and Labels to remove any schema-wise differences. Specifically, this implies renaming the 'key' column to 'key_name'.

Decision
--------

1. Create a view for each annotation table aliasing the 'key' column as 'key_name' on the DB rather than the Model.
1. Modify our code, tests and model to rely on the view and the 'key_name' column.
1. Rename the 'key' column to 'key_name' for each annotation table.
1. Remove the views on annotation tables.

It is important to note:

- Steps 1 and 2 are backward compatible with any CC version,
- Step 3 is only backward compatible with any CC version incorporating changes from steps 1 and 2,
- Step 4 is only backward compatible with any CC version featuring changes from step 3.

Consequences
------------

### Positive Consequences

1. Consolidation of the codebase for labels and annotations and the alignment of database table layouts. This will simplify and standardize the current setup which can be confusing due to the table aliases in the models.

### Negative Consequences

1. A staged rollout will be necessary requiring clear documentation about limitations on upgrade paths since we have two changes that necessitate a specific minimum CF-Deployment/CAPI version.

0 comments on commit 218f103

Please sign in to comment.