Prevent unnecessary jsonb updates for builds_metadata
What does this MR do and why?
While investigating gitlab-com/gl-infra/production#7907 (closed) we've noticed that the requests for job assignment to /api/:version/jobs/request
were updating the config_options
column on ci_builds_metadata
table:
Process 3091143: /*application:web,correlation_id:eb6a2d49457ed84be0c62fb87dbf97cf,endpoint_id:POST /api/:version/jobs/request,db_config_name:ci*/ UPDATE "ci_builds_metadata" SET "config_options" = '[REDACTED]' WHERE "ci_builds_metadata"."id" = 3028893490
This should not happen because we don't have any business logic to change the values stored in that column after a pipeline is created.
This is started by transitioning the job into the running state which triggers a transition callback:
after_transition pending: :running do |build|
build.ensure_metadata.update_timeout_state
end
This callback calls update on the Ci::BuildMetadata
record:
def update_timeout_state
timeout = timeout_with_highest_precedence
return unless timeout
update(timeout: timeout.value, timeout_source: timeout.source)
end
Accessing the record would trigger config_options
to be registered as changed:
[1] pry(main)> m.assign_attributes(timeout: 120, timeout_source: 1)
=> nil
[2] pry(main)> m.changes
=> {"timeout"=>[nil, 120],
"config_options"=>
[{"image"=>{"name"=>"busybox:latest"},
"script"=>["echo \"Do another parallel test here\"", "echo \"For example run a lint test\""],
"after_script"=>["echo \"After script section\"", "echo \"For example you might do some cleanup here\""],
"before_script"=>["echo \"Before script section\"", "echo \"For example you might run an update here or install a build dependency\"", "echo \"Or perhaps you might print out some debugging details\""]},
{"image"=>{"name"=>"busybox:latest"},
"script"=>["echo \"Do another parallel test here\"", "echo \"For example run a lint test\""],
"after_script"=>["echo \"After script section\"", "echo \"For example you might do some cleanup here\""],
"before_script"=>["echo \"Before script section\"", "echo \"For example you might run an update here or install a build dependency\"", "echo \"Or perhaps you might print out some debugging details\""]}]}
[3] pry(main)> m.changes["config_options"].reduce(:==)
=> true
If the timeout
and timeout_source
values do not change, we end up updating only the serialized JSONb values.
Screenshots or screen recordings
Before
[1] pry(main)> metadata = Ci::BuildMetadata.last; nil
Ci::BuildMetadata Load (0.9ms) SELECT "ci_builds_metadata".* FROM "ci_builds_metadata" ORDER BY "ci_builds_metadata"."id" DESC LIMIT 1 /*application:console,db_config_name:ci,line:(pry):1:in `__pry__'*/
=> nil
[2] pry(main)> metadata.changes
=> {}
[3] pry(main)> metadata.config_options; nil
=> nil
[4] pry(main)> metadata.changes.keys
=> ["config_options"]
After
[2] pry(main)> metadata = Ci::BuildMetadata.last; nil
Ci::BuildMetadata Load (0.7ms) SELECT "ci_builds_metadata".* FROM "ci_builds_metadata" ORDER BY "ci_builds_metadata"."id" DESC LIMIT 1 /*application:console,db_config_name:ci,line:(pry):2:in `__pry__'*/
=> nil
[3] pry(main)> metadata.changes
=> {}
[4] pry(main)> metadata.config_options; nil
=> nil
[5] pry(main)> metadata.changes.keys
=> []
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.