[ 
https://issues.apache.org/jira/browse/CASSANDRA-13457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16389371#comment-16389371
 ] 

mck edited comment on CASSANDRA-13457 at 3/7/18 11:27 AM:
----------------------------------------------------------

hey [~spo...@gmail.com], looks all awesome, so a +1 from me.
 i do have some small questions/thoughts…

 

{{diagnostic_events_enabled: true}}
 - After recent discussions on the dev ML around the use of experimental flags, 
eg on MV, would it make more sense that this was false by default?

 
 {{DiagnosticEvent.addIfNotNull(..)}}
 - this seems it could be a bit trivial… Can we just enforce toString 
serialisation in the subclasses instead? (like Hint does it) 

 
 {{DiagnosticEventService}}
 - Can we avoid the static fields? So to be avoiding adding to the 
CASSANDRA-7837 problems… I don't think C* has a better habit in place for this? 
But a singleton would be one better than all static fields…
 - Not too sure why a number of fields in existing classes were changed from 
private to package-protected, for example in Gossiper. If it's for tests in 
latter branches should they deserve the @VisibleForTesting annotation? And 
should that change also happen in the latter branches        
 - Should the event classes be included in the client jarfile (or some 'type 
and deserialisation' part of the event classes). This would then introduce 
issues of compatibility, (eg enums). If event classes are not exposed 
client-side, could they then be package private? (they're not used outside 
their package)
 - What about conglomeration between metrics, diag events, and tracing events? 
For example i wonder if the latter two will often pair?, good example in 
HintsDispatcher

 

 


was (Author: michaelsembwever):
hey [~spo...@gmail.com], looks all awesome, so a +1 from me.
 i do have some small questions/thoughts…

 

{{diagnostic_events_enabled: true}}
 - After recent discussions on the dev ML around the use of experimental flags, 
eg on MV, would it make more sense that this was false by default?

 
 {{DiagnosticEvent.addIfNotNull(..)}}
 - this seems it could be a bit trivial… Can we just enforce toString 
serialisation in the subclasses instead? (like Hint does it) 

 
 {{DiagnosticEventService}}
 - Can we avoid the static fields? So to be avoiding adding to the 
CASSANDRA-7837 problems… I don't think C* has a better habit in place for this? 
But a singleton would be one better than all static fields…
 - Not too sure why a number of fields in existing classes were changed from 
private to package-protected, for example in Gossiper. If it's for tests in 
latter branches should they deserve the @VisibleForTesting annotation? And 
should that change also happen in the latter branches        
 - Should the event classes be included in the client jarfile (or some 'type 
and deserialisation' part of the event classes). This would then introduce 
issues of compatibility, (eg enums). If event classes are not exposed 
client-side, could they then be package private? (they're not used outside 
their package)
 - What about conglomeration between metrics, diag events, and tracing events? 
For example when would the latter two not ever pair?, good example in 
HintsDispatcher

 

 

> Diag. Events: Add base classes
> ------------------------------
>
>                 Key: CASSANDRA-13457
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13457
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core, Observability
>            Reporter: Stefan Podkowinski
>            Assignee: Stefan Podkowinski
>            Priority: Major
>
> Base ticket for adding classes that will allow you to implement and subscribe 
> to events.



--
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

Reply via email to