storage: fix dyncfg updates for future clusters #26194
Conversation
guswynn
added
the
release-blocker
guswynn
force-pushed
the
fix-dyncfg-storage
branch
from
7db9d54 to
20032ed
Compare
| .storage_configuration | ||
| .update(storage_parameters); | ||
|
|
||
| // Clear out the updates as we no longer forward them to anyone else to process. |
There was a problem hiding this comment.
@rjobanp maybe I should just not do this, it only saves one set of updates per-worker
There was a problem hiding this comment.
Oh actually we clone self.storage_state.storage_configuration a bunch during rendering...
guswynn
force-pushed
the
fix-dyncfg-storage
branch
from
20032ed to
e54db90
Compare
rjobanp
left a comment
rjobanp
left a comment
There was a problem hiding this comment.
LGTM to get in for this release. It would be great to add some tests around this that validate dyncfg update propagation across clusters and to new clusters, etc in a future PR
|
@rjobanp definitely, but we would need to think of a way to expose parameter values in an testable way...maybe through the internal http endpoint? |
Metrics! |
|
Oh @danhhz thats a good idea, a configset metric with a name label |
3 participants
I was being wrong with this option:
Roshan found #26189, and #26131 made me realize that future clusters wont get the new updates.
Motivation
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.