glusterfs
glusterfs copied to clipboard
Multiple files: reduce redirection for measure_latency variable
As far as I can see, we have the ctx variable in the call_stack_t structure only to be able to access the measure_latency boolean. Example from create_frame():
if (frame->root->ctx->measure_latency) {
timespec_now(&stack->tv);
memcpy(&frame->begin, &stack->tv, sizeof(stack->tv));
}
I did not see any other use for the ctx variable apart from that boolean. We can instead just copy that boolean and reduce one level of indirection (which seem to be happening in create_frame(), FRAME_DESTROY() and STACK_UNWIND_STRICT macro)
Updates: #1874 Signed-off-by: Yaniv Kaul [email protected]
/run regression
0 test(s) failed
1 test(s) generated core ./tests/bugs/glusterd/mgmt-handshake-and-volume-sync-post-glusterd-restart.t
7 test(s) needed retry ./tests/000-flaky/basic_afr_split-brain-favorite-child-policy.t ./tests/000-flaky/basic_mount-nfs-auth.t ./tests/000-flaky/bugs_nfs_bug-1116503.t ./tests/basic/afr/ta-shd.t ./tests/bugs/bitrot/bug-1245981.t ./tests/bugs/changelog/bug-1208470.t ./tests/bugs/glusterd/bug-1696046.t
2 flaky test(s) marked as success even though they failed ./tests/000-flaky/basic_mount-nfs-auth.t ./tests/000-flaky/bugs_nfs_bug-1116503.t https://build.gluster.org/job/gh_centos7-regression/2593/
/run regression
I think this patch has a problem: by default
measure_latency
is true for all xlators, butglobal_xl_reconfigure()
only modifies the value for the global xlator. So I don't see how the option can be disabled.
Perhaps I need to change glusterfs_graph_insert() and glusterfs_graph_prepare() to inherit the value instead of just setting it to true.
I think this patch has a problem: by default
measure_latency
is true for all xlators, butglobal_xl_reconfigure()
only modifies the value for the global xlator. So I don't see how the option can be disabled.Perhaps I need to change glusterfs_graph_insert() and glusterfs_graph_prepare() to inherit the value instead of just setting it to true.
The problem with this is that the value won't be modifiable at runtime. This option is commonly used to enable profiling temporarily to debug some issue. If we don't allow a dynamic change, a volume restart would be needed to enable profiling.
An alternative is to only keep measure_latency
in the stack, and set it from the ctx->measure_latency
at stack creation. Then all STACK_* macros can use it instead of accessing ctx. I think this covers most of its use cases, at least the most important ones.
Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.
Closing this issue as there was no update since my last update on issue. If this is an issue which is still valid, feel free to open it.