construct service once and have it shared by tchannel and http servers

[Haijuncao]

No description provided.

[richardartoul]

Mostly LGTM, just two comments:

Hard to tell from C.I but you have these minor lint issues which is why its failing:

src/dbnode/network/server/httpjson/node/server.go:38:2:warning: unused struct field github.com/m3db/m3/src/dbnode/network/server/httpjson/node.server.ttopts (structcheck)
--
  | src/dbnode/network/server/httpjson/node/server.go:38:2:warning: field ttopts is unused (U1000) (unused)

Also, now that we've fixed this, could you bump the size of the the constant writeBatchPooledReqPoolSize from 1024 to 4096? The reason I discovered this issue is I wanted to make that constant bigger and then I noticed we were allocating the pools twice.

Ideally the pool size would be controlled by config, but changing the constant is good enough for now.

[codecov]

Codecov Report

Merging #1420 into master will increase coverage by 0.2%.
The diff coverage is 88.8%.

@@           Coverage Diff            @@
##           master   #1420     +/-   ##
========================================
+ Coverage    70.6%   70.8%   +0.2%     
========================================
  Files         835     833      -2     
  Lines       71788   71492    -296     
========================================
- Hits        50695   50650     -45     
+ Misses      17782   17534    -248     
+ Partials     3311    3308      -3

Flag	Coverage Δ
#aggregator	82.3% <ø> (ø)	⬆️
#cluster	85.8% <ø> (ø)	⬆️
#collector	63.7% <ø> (ø)	⬆️
#dbnode	80.7% <88.8%> (+0.7%)	⬆️
#m3em	73.2% <ø> (ø)	⬆️
#m3ninx	74.2% <ø> (ø)	⬆️
#m3nsch	51.1% <ø> (ø)	⬆️
#metrics	17.6% <ø> (ø)	⬆️
#msg	74.9% <ø> (-0.2%)	⬇️
#query	65.6% <ø> (-0.1%)	⬇️
#x	76% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75661bc...c0bb1dd. Read the comment docs.

[codecov]

Codecov Report

Merging #1420 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1420   +/-   ##
======================================
  Coverage    70.9%   70.9%           
======================================
  Files         834     834           
  Lines       71677   71677           
======================================
  Hits        50830   50830           
  Misses      17526   17526           
  Partials     3321    3321

Flag	Coverage Δ
#aggregator	82.3% <0%> (ø)	⬆️
#cluster	85.8% <0%> (ø)	⬆️
#collector	63.7% <0%> (ø)	⬆️
#dbnode	80.8% <0%> (ø)	⬆️
#m3em	73.2% <0%> (ø)	⬆️
#m3ninx	74.2% <0%> (ø)	⬆️
#m3nsch	51.1% <0%> (ø)	⬆️
#metrics	17.6% <0%> (ø)	⬆️
#msg	75% <0%> (ø)	⬆️
#query	65.7% <0%> (ø)	⬆️
#x	76% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7689e2...26440ce. Read the comment docs.

[Haijuncao]

Mostly LGTM, just two comments:

Hard to tell from C.I but you have these minor lint issues which is why its failing:

src/dbnode/network/server/httpjson/node/server.go:38:2:warning: unused struct field github.com/m3db/m3/src/dbnode/network/server/httpjson/node.server.ttopts (structcheck)
--
  | src/dbnode/network/server/httpjson/node/server.go:38:2:warning: field ttopts is unused (U1000) (unused)

Also, now that we've fixed this, could you bump the size of the the constant writeBatchPooledReqPoolSize from 1024 to 4096? The reason I discovered this issue is I wanted to make that constant bigger and then I noticed we were allocating the pools twice.

Ideally the pool size would be controlled by config, but changing the constant is good enough for now.

I will make another diff for change default pool size. Maybe also make the pool size configurable.