[
https://issues.apache.org/jira/browse/CASSANDRA-14677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16596951#comment-16596951
]
Jason Brown commented on CASSANDRA-14677:
-----------------------------------------
I took a decent look at the patch provided.
Previously, we had memoized {{auditLogEnabled}} in the parent {{Request}} class
in order read the volatile {{auditLogManager.isAuditingEnabled()}} only once
per instance.
With this refactor, you are calling {{auditLogManager.isAuditingEnabled()}}
everytime you need to reference the variable. You might consider memoizing the
value again.
Also, {{Request.perform()}} is an unexpected naming choice, and doesn't seem
typical of how we usually name things. You should add a comment that
{{perform()}} is now the main entry point for running the {{Request}}, and
perhaps make {{execute()}} protected (instead of public).
I think it would be helpful for committers and for future reviewers to have a
better understanding of what is meant by "big mess". Perhaps you could update
the description to better outline the specific issues with the {{execute()}}
method implementations. This would also make the changes in the patch more
clear for review.
> Clean up Message.Request implementations
> ----------------------------------------
>
> Key: CASSANDRA-14677
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14677
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Priority: Minor
> Fix For: 4.0.x
>
>
> First tracing support, many years ago, then most recently audit log, made a
> big mess out of {{Message.Request.execute()}} implementations.
> This patch tries to clean up some of it by removing tracing logic from
> {{QueryState}} and moving shared tracing functionality to
> {{Message.Request.perform()}}. It also moves out tracing and audit log boiler
> plate into their own small methods instead of polluting {{execute()}}
> implementations.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
