thangTang commented on PR #5072:
URL: https://github.com/apache/hbase/pull/5072#issuecomment-1452931997
> Previously this duplication was nicely encapsulated in
MetricsRegionServer. We passed in a TableName, and it updated table metrics if
appropriately.
>
> You removed the TableName argument, so now the code has to live outside
MetricsRegionServer. What if you replace TableName argument with Region
argument?
Agree that is a truly problem. Honestly I have consider it too.
I think the previous code structure is a bit confusing.
From the naming point of view, `MetricsRegionServer` and
`MetricsTableRequests` should be at the same level, but they are actually
completely different. We can see that `MetricsRegionServer` holds
`serverSource`, `metricsTable`, `tableMetrics`, `userAggregate`, and so on.
My vision is that each metrics object is only responsible for operating one
type of metrics (that is, a bean in jmx), so that the metric of the table
belongs to the metricsTable, the metric of the server belongs to the
metricsServer, and the metric of the region belongs to the metricsRegion. Then
there is a tool class that only needs to be called once externally (such as in
RSRpcServices), and encapsulates all operations on metrics in the methods of
this tool class.
In fact, the current `MetricsRegionServer` is such a tool class. We can
change its name, or create a new class, and then gradually refactor various
metrics, transfer the function of the main entrance to the new class, and let
`MetricsRegionServer` gradually degenerate into only responsible for
serverMetrics.
But in order to disassemble the work and try to have a clear goal, in this
ticket I don't intend to modify anything other than tableLatencies. That's why
I just left it here.
After all now I'm glad to temporarily put these logics back into
`MetricsRegionServer`, just as you said, replace TableName argument with Region
argument. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
