feat(MOS): support OpenMedia's hot standby #1169
Conversation
jstarpl
changed the title
jstarpl
added
the
Contribution from BBC
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release51 #1169 +/- ##
=============================================
- Coverage 57.94% 57.87% -0.08%
=============================================
Files 523 526 +3
Lines 84719 84969 +250
Branches 4415 4303 -112
=============================================
+ Hits 49094 49172 +78
- Misses 35570 35744 +174
+ Partials 55 53 -2 ☔ View full report in Codecov by Sentry. |
…nnections are offline
There was a problem hiding this comment.
I'm not keen on the name 'hot standby', I find that confusing.
I would expect the 'hot standby' mode to be the one where there is a 'hot spare' connection maintained the whole time, not when the connection is 'cold' except when the primary is down.
I would prefer that the field was renamed to be more descriptive to how the connection will behave, and the description could include something like For OpenMedia Hot Standby to make it clear it should be used with this
…tStandby to openMediaHotStandby
|
|
||
| private _messageQueue: Queue | ||
|
|
||
| constructor(parent: CoreHandler, mosDevice: IMOSDevice, mosHandler: MosHandler) { | ||
| constructor(parent: CoreHandler, mosDevice: IMOSDevice, mosHandler: MosHandler, openMediaHotStandby: boolean) { |
There was a problem hiding this comment.
I'm wondering if this should be put into an options object? Carrying these opaque flags enabling out-of-spec behavior seems like something that will get problematic very quickly. What about?
| constructor(parent: CoreHandler, mosDevice: IMOSDevice, mosHandler: MosHandler, openMediaHotStandby: boolean) { | |
| constructor(parent: CoreHandler, mosDevice: IMOSDevice, mosHandler: MosHandler, opts?: { openMediaHotStandby? : boolean }) { |
| async registerMosDevice( | ||
| mosDevice: IMOSDevice, | ||
| mosHandler: MosHandler, | ||
| openMediaHotStandby: boolean |
There was a problem hiding this comment.
Same as above - I understand that these additional options are necessary to pass around, but maybe we could group these flags into some sort of an options object where their meaning will be available, and not as just boolean arguments into functions?
| openMediaHotStandby: boolean | |
| opts?: { openMediaHotStandby?: boolean } |
| @@ -482,6 +488,11 @@ export class MosHandler { | |||
| deviceOptions.primary.heartbeatInterval = | |||
| deviceOptions.primary.heartbeatInterval || DEFAULT_MOS_HEARTBEAT_INTERVAL | |||
|
|
|||
| if (deviceOptions.secondary?.id && this._openMediaHotStandby[deviceOptions.secondary.id]) { | |||
| //@ts-expect-error this is not yet added to the official mos-connection | |||
| deviceOptions.secondary.openMediaHotStandby = true | |||
There was a problem hiding this comment.
Does this mean that this PR is not mergable yet? Because there needs to be an update to mos-connection before?
About the Contributor
This PR is made on behalf of BBC
Type of Contribution
This is a Feature
Current Behavior
Currently the Primary and Secondary MOS connection are expected to be online at the same time.
New Behavior
OpenMedia treats the Secondary server as a Hot Standby, so it's not connected while Primary server is connected.
To handle this, a "Hot Spare" option is added in settings to the Secondary server.
When hot spare is activated, status messages are:
Testing Instructions
Connect MOS gateway to a 2 server OpenMedia setup.
Time Frame
We intend to finish the development on this feature in two weeks time.
Other Information
Status