Skip to content

etcd all about when hung up, the user can use vtgate normal#3640

Closed
yangxuanjia wants to merge 23 commits intovitessio:masterfrom
yangxuanjia:master
Closed

etcd all about when hung up, the user can use vtgate normal#3640
yangxuanjia wants to merge 23 commits intovitessio:masterfrom
yangxuanjia:master

Conversation

@yangxuanjia
Copy link
Copy Markdown
Contributor

etcd all about when hung up, the user can use vtgate normal, to ensure that these operations such as dml, create a new connection, use db, show database can be used normally, leave us Enough time to recover etcd cluster


// If it is not time to check again, then return either the cached
// value or the cached error
cacheValid := entry.value != nil && time.Since(entry.insertionTime) < server.cacheTTL
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't access any value inside entry if you don't have the entry.mutex.Lock()...

@demmer
Copy link
Copy Markdown
Member

demmer commented Feb 8, 2018

As mentioned in slack I have a PR that does much of this and also respects the refreshInterval and handles concurrency. I am in the middle of tidying it up and will put it up for review shortly.

@demmer
Copy link
Copy Markdown
Member

demmer commented Feb 8, 2018

At the same time I noticed that if the topo is slow or unresponsive the debug ui could be locked out, so I also think we should drop the lock before calling into the topo server.

@alainjobart
Copy link
Copy Markdown
Contributor

But we still shouldn't have two outgoing calls going on at the same time for the same thing, and that's another thing the mutex protects. So maybe we need a second mutex, guarding the call to the topo service, so only one happens at the same time? Or have a state variable, that describes a thread is asking for the data, and a channel that other threads can synchronize on waiting for the answer? This might get a bit too complicated...

@demmer
Copy link
Copy Markdown
Member

demmer commented Feb 8, 2018

@yangxuanjia @alainjobart I put up #3641 which I believe addresses the core issue raised here along with Alain's concerns. It has slightly different semantics from this proposal.

Please let me know what you both think.

@yangxuanjia yangxuanjia closed this Feb 9, 2018
@yangxuanjia
Copy link
Copy Markdown
Contributor Author

demmer's PR contains the idea of #3640 and better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants