Moved creation of topo.Server outside of tx_throttler. #2568
Conversation
|
You didn't have to change vttablet.go / vtcombo.go? |
erzel
force-pushed
the
passing_topo_server_to_tx_throttler
branch
from
f254e39
to
787661f
Compare
|
Oops. You're right. I've updated these files. I've code searched for all other callers of NewServer / NewTabletServer and couldn't find any others. PTAL. |
| @@ -74,7 +74,8 @@ func main() { | |||
| } | |||
|
|
|||
| // creates and registers the query service | |||
| qsc := tabletserver.NewServer() | |||
| ts := topo.Open() | |||
There was a problem hiding this comment.
Is this function idempotent, or can it be made so? If so, we can have TabletServer start this as a sub-service internally, which will be in line with the rest of the sub-services. We'll also reduce one dependency.
There was a problem hiding this comment.
I don't think the function is idempotent, currently. It looks to me like it opens an RPC connection.
There was a problem hiding this comment.
This function is creating a new object, it should actually be called 'New'. We used to have a single 'global' topo server, and that was a bad pattern that made a whole bunch of tests really hard to manage.
|
Change looks good, but can you make sure the unit tests pass locally before review? thanks! |
erzel
force-pushed
the
passing_topo_server_to_tx_throttler
branch
from
787661f
to
370c55f
Compare
|
Sorry Alain. I'll make sure the tests are passing next time. PTAL. |
erzel
force-pushed
the
passing_topo_server_to_tx_throttler
branch
from
370c55f
to
0bdc2d2
Compare
* Parse Insights flags with pflags This allows us to deprecate `-insights_reorder_threshold` properly Signed-off-by: Patrick Reynolds <patrick@piki.org> * Don't expect Insights flags for vtexplain Insights flags are only valid for vtgate. Signed-off-by: Patrick Reynolds <patrick@piki.org> --------- Signed-off-by: Patrick Reynolds <patrick@piki.org>
@alainjobart PTAL.