-
Notifications
You must be signed in to change notification settings - Fork 17
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
metrics: factor out TLS cache #166
Conversation
lib/metrics/metrics-tls-cache.c
Outdated
@@ -84,15 +84,15 @@ _deinit_tls_clusters_map_thread_init_hook(gpointer user_data) | |||
} | |||
|
|||
static void | |||
_init_tls_clusters_map_apphook(gint type, gpointer user_data) | |||
_init_tls_clusters_map_apphook(void) |
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.
If I understand correctly, this is not an apphook anymore, so maybe remove that from the name.
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.
Thanks, fixed.
@@ -144,14 +144,13 @@ metrics_tls_cache_get_labels_len(void) | |||
void | |||
metrics_tls_cache_global_init(void) |
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.
maybe this is a bit too granular for apphook, and we should just have a metrics_global_init()/deinit() functions.
e.g. drop the tls_cache piece from the name. if there will be multiple metrics related global init/deinit chores, we could call all of those from a single metrics_global_init() function.
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.
Agree, fixed.
lib/metrics/metrics-tls-cache.c
Outdated
} | ||
|
||
StatsClusterLabel * | ||
metrics_tls_cache_get_next_label(void) |
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.
[optional] this is more like an alloc() function. get_next_label() seems like a read only operation. I needed to check out the implementation to understand what it was doing exactly.
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.
Good idea, fixed.
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
e116770
to
6a0edda
Compare
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
6a0edda
to
f890a2f
Compare
No description provided.