-
Notifications
You must be signed in to change notification settings - Fork 831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make MetricsFeature's dependencies explicit #20945
Conversation
33fcbb4
to
0799185
Compare
void MetricsFeature::prepare() { | ||
_queryRegistryFeature = std::move(_lazyQueryRegistryFeatureRef).get(); | ||
_statisticsFeature = std::move(_lazyStatisticsFeatureRef).get(); | ||
_engineSelectorFeature = std::move(_lazyEngineSelectorFeatureRef).get(); | ||
_clusterMetricsFeature = std::move(_lazyClusterMetricsFeatureRef).get(); | ||
_clusterFeature = std::move(_lazyClusterFeatureRef).get(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we assert somehow that prepare()
and therefore get()
methods are really called only once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is asserted that get()
is called only once: https://github.com/arangodb/arangodb/pull/20945/files?show-viewed-files=true&file-filters%5B%5D=#diff-ba7c32ee3716b0719387545111a837bd0a213f052957a14db2d32b9ee24be3ccR103
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Scope & Purpose
Make the dependencies of MetricsFeature explicit, so it can get by without getFeature calls, so it can be an ApplicationFeature instead of an ArangodFeature, so it can be used separately from it.
In order to achieve that, this PR introduces LazyApplicationFeatureReference to handle potential circular references in the feature dependencies (which we should try to get rid of later, whenever possible).
Related Information