[ 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org