CBG-3382: Catch invalid state transition in asyncDatabaseOnline #6392
Conversation
bbrks
changed the title
rest/server_context.go
Outdated
| // 2nd atomic might end up being Starting here if there's a legitimate race, but it's the most we can do for CAS | ||
| panic(fmt.Sprintf("database state wasn't Starting during asyncDatabaseOnline Online transition... now %q", db.RunStateString[atomic.LoadUint32(&dbc.State)])) |
There was a problem hiding this comment.
Should we PanicfCtx and say that we can't do anything to recover from this state in the comment?
We can't take down a database because it's already coming online, so the best we can do is take down SG?
Or do you think this is unlikely to get hit and not worth panicking about?
There was a problem hiding this comment.
Hmm... This was mostly to catch it more obviously via test if we decided to forget another return again. I'm not sure what we should do really. I don't expect this to end up panicing in a released version, but I might expect it in development/testing.
I agree about at least using PanicfCtx over panic though. Better chance of getting that logging captured.
| @@ -934,7 +934,7 @@ func (sc *ServerContext) asyncDatabaseOnline(nonCancelCtx base.NonCancellableCon | |||
|
|
|||
| if !atomic.CompareAndSwapUint32(&dbc.State, db.DBStarting, db.DBOnline) { | |||
| // 2nd atomic might end up being Starting here if there's a legitimate race, but it's the most we can do for CAS | |||
| panic(fmt.Sprintf("database state wasn't Starting during asyncDatabaseOnline Online transition... now %q", db.RunStateString[atomic.LoadUint32(&dbc.State)])) | |||
| base.PanicfCtx(ctx, "database state wasn't Starting during asyncDatabaseOnline Online transition... now %q", db.RunStateString[atomic.LoadUint32(&dbc.State)]) | |||
There was a problem hiding this comment.
Does ctx here have the database name? If so, that's a cool benefit of PanicfCtx.
If it doesn't, can you add the database name in the error?
Spun out of #6388
CBG-3382
StartOnlineProcessesfromasyncDatabaseOnlinethat causes an invalid state transition via missedreturnatomic.CompareAndSwapIntegration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/1989/