Move getter/setter methods to Ci::Metadatable
module and apply feature flags
Problem
The code relies strictly on data to be written and read on specific tables. For example exit_code=
always delegates to Ci::BuildMetadata
. Since some data is moved to new destinations (e.g. intrinsic data moved to ci_builds
and immutable processing data to ci_job_prototypes
) we need to be able to use feature flags to read/write data to specific destination.
Proposal
Move getter/setter methods from Ci::Build
and Ci::BuildMetadata
to Metadatable
module so we can switch the source based on the feature flag / state (options, yaml_variables, timeout, id_tokens, exit_code, etc.).
By leveraging Metadatable
we would depend upon the abstraction rather than the specific table/model Ci::Build
or Ci::BuildMetadata
.
Checkout the PoC Draft: PoC: split build metadata into `job_prot... (!193948 - closed) to see what methods need to be moved and how. Methods like timeout_human_readable
, timeout_value
, exit_code
, exit_code=
, enable_debug_trace!
, etc. needs to be moved.
The idea is to have:
- getters will use a feature flag
read_from_new_ci_destinations
to determine whether to serve the value from the newci_job_prototypes
table or from the legacyci_builds_metadata
. - setters will write to 2 destinations:
-
either
ci_builds
for intrinsic data orci_job_prototypes
for immutable processing data. -
ci_builds_metadata
to maintain behavior backwards compatible and allowing us to make 2-way-door decisions. We will use a feature flagstop_writing_builds_metadata
to control when to stop writing to this table.
-
Implementation
read_from_new_ci_destinations
.
1. Introduce the feature flag - Apply to
options
andyaml_variables
. - Update all places that preload
:metadata
to also preload:job_definition
, to avoid N+1 queries.
-> MR: Introduce FF `read_from_new_ci_destinations` an... (!199971 - merged)
stop_writing_builds_metadata
for job processing data.
2. Prepare for introduction of FF - Update job factories to write to both
ci_builds_metadata
andci_job_definitions
. Stop mutatingoptions
andyaml_variables
in spec tests. - We'll need to update the
degenerate!
method since all factory-created job objects will havejob_definition
present, so we need to destroy it. - Update all factories/specs affected per !199971 (comment 2675309188):
@fabiopitino
:@lma-git
- in a discussion with@mbobin
I recall we discussed this problem. We cannot mutate the config inci_job_definitions
because it would impact other jobs sharing the same job definition. E.g. 2 jobs A and B are initially created with the same options/definition. They will share the same definition ID. If later we mutate job A's options and update the relative job definition, it would mutate also the definition of job B.IMO we should gradually remove all
options=
from the specs and replace them with factories or stubs likeexpect(job).to receive(:options).and_return(...)
.
-> MR: Update Rspec job factories to write to ci_job_d... (!202317 - merged)
stop_writing_builds_metadata
.
3. Introduce the feature flag - Apply the feature flag to an intrinsic data field to start.
-> MR: Introduce FF `stop_writing_builds_metadata` and... (!202462 - merged)
Ci::Metadatable
. Apply feature flags as necessary.
4. Address each field that needs to have its getter/setter added/moved/updated to Field(s) | Status | MR / Notes |
---|---|---|
options , yaml_variables
|
!199971 (merged), !203321 (merged) | |
scoped_user_id |
!203085 (merged) | |
timeout , timeout_source
|
#551827 (closed), !202970 (merged) | |
exit_code |
!202815 (merged) | |
debug_trace_enabled |
!202462 (merged) | |
downstream_errors |
Now saved to ci_job_messages table, under the feature flag ci_new_downstream_errors_location . See !202194 (merged). |
|
interruptible |
!203212 (merged) | |
secrets |
!202968 (merged) | |
id_tokens |
!202681 (merged) |