refactor: More cleanup of server/start.go #16238
Conversation
github-prbot
requested review from
a team,
alexanderbez and
likhita-809
and removed request for
a team
|
Are we sure this orjitech leaking resource linter is right? I fail to see how this code change can have a leak whereas the old code didn't. (Also it can't actually be a leak in context, as this function's termination means program termination...) Please let me know if this is a leakage, I don't see it! If not, how do I suppress the error here? |
| if err != nil { | ||
| return err | ||
| } | ||
| defer appCleanupFn() |
There was a problem hiding this comment.
The tracestore defer is moved up here, which is equivalent to before with startInProcess, as the tracestore was cleaned up after the tmNode.
(It wasn't cleaned up for startStandAlone at all before)
|
#16208 I had a similar pr here, seems to be a subset of your pr |
| @@ -542,6 +519,7 @@ func startAPIServer(ctx context.Context, g *errgroup.Group, cmtCfg *cmtcfg.Confi | |||
| } | |||
| // TODO: Why do we reload and unmarshal the entire genesis doc in order to get the chain ID. | |||
| // surely theres a better way. This is likely a serious node start time overhead. | |||
| // Shouldn't it be in cmtCfg.ChainID() ? | |||
There was a problem hiding this comment.
the genesis and config can have the incorrect values if a user is syncing from a snapshot, im not sure if this is needed
There was a problem hiding this comment.
Should we move this into a follow-up issue for StartAPIServer: how to get chain-id ?
|
@yihuang so sorry I missed that, I would've just waited! Looks like both PR's do independently helpful things, I think we should get both sets of changes in |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Description
Address some TODO's I left in server/start.go from #15041
This PR has five changes, beyond some line count / complexity reduction. Namely:
These weren't done in #15041 in order for that PR to be changing no functionality.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change