simplify graph signal handler #3542
Conversation
heroicNeZha
added
ready-for-testing
heroicNeZha
force-pushed
the
graphd_block
branch
from
45c0e97 to
af0e772
Compare
src/daemons/GraphDaemon.cpp
Outdated
| serverStatus_.store(STATUS_RUNNING); | ||
| FLOG_INFO("Starting nebula-graphd on %s:%d\n", localhost.host.c_str(), localhost.port); | ||
| try { | ||
| gServer->serve(); // Blocking wait until shut down via gServer->stop() |
There was a problem hiding this comment.
I think you just need call serve in this thread.
There was a problem hiding this comment.
When I call serve(), the deamon will be blocked here, I don’t know how to ensure that the stop call is not a worker thread
src/daemons/GraphDaemon.cpp
Outdated
| enum ServiceStatus { STATUS_UNINITIALIZED = 0, STATUS_RUNNING = 1, STATUS_STOPPED = 2 }; | ||
| std::atomic<ServiceStatus> serverStatus_{STATUS_UNINITIALIZED}; | ||
| std::mutex muStop_; | ||
| std::condition_variable cvStop_; |
There was a problem hiding this comment.
Prefer to provide a class to handle this signal process
heroicNeZha
force-pushed
the
graphd_block
branch
from
af0e772 to
8ce57b9
Compare
src/graph/service/GraphServer.cpp
Outdated
| @@ -24,7 +24,7 @@ bool GraphServer::start() { | |||
| int numThreads = | |||
| FLAGS_num_worker_threads > 0 ? FLAGS_num_worker_threads : gServer_->getNumIOWorkerThreads(); | |||
| std::shared_ptr<apache::thrift::concurrency::ThreadManager> threadManager( | |||
| PriorityThreadManager::newPriorityThreadManager(numThreads, false /*stats*/)); | |||
There was a problem hiding this comment.
After updating third party 3.0，newPriorityThreadManager remove second argument
There was a problem hiding this comment.
This argument affects the performance a lot, we must know how it behaves now. And make sure if it has another way to set the value. A benchmark must done for this change.
There was a problem hiding this comment.
facebook/fbthrift@bfb527c?diff=split
Through this commit，thrift removed stats, and the function is the same as setting it to false
| } | ||
|
|
||
| ServiceStatus serverExpected = ServiceStatus::STATUS_RUNNING; | ||
| serverStatus_.compare_exchange_strong(serverExpected, STATUS_STOPPED); |
There was a problem hiding this comment.
Don't wait it to be running?
There was a problem hiding this comment.
it wait in thriftServer.stop
heroicNeZha
force-pushed
the
graphd_block
branch
from
153d115 to
ffab7f9
Compare
What type of PR is this?
What does this PR do?
closes #3441
simplify graphd signal handler：Set stop condition instead of waiting all workers stop
Which issue(s)/PR(s) this PR relates to?
In #3437 , it simplify storage SIGINT signal handler
Special notes for your reviewer, ex. impact of this fix, etc:
Additional context/ Design document:
Checklist:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: