[
https://issues.apache.org/jira/browse/CASSANDRA-13457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16573293#comment-16573293
]
Stefan Podkowinski commented on CASSANDRA-13457:
------------------------------------------------
"Config.diagnostic_events_enabled needs to be volatile if you will modify it at
runtime via JMX."
Will amend before committing, thx!
"nit: TokenAllocatorEvent's ctor has a huge parameter list."
One of the requirements for raising events was to generate as little garbage as
possible. Some events, such as SchemaEvents, are rare enough to justify
additional object allocations, but I'd also like to keep class design
consistent and not having to use a constructor based approach for more frequent
events again.
"DiagnosticEventService.publish() is invoked on the same thread as the caller."
I'd expect devs to be aware of this when creating new consumers and not block
the caller. E.g. the DiagnosticEventMemoryStore uses a concurrent map for this.
This is a non-pluggable part of the code (at least before CASSANDRA-13460) and
I can't think of any way we suddenly would have consumers that would be
implemented without seeing this issue.
"Please document why the map's value type of the return from
DiagnosticEvent.toMap() is Serializable."
Will add some more docs, but really this method is only supposed to return
details on the event that can be used by external consumers. Best to just look
at some of the existing implementations to get a better idea on that.
"I'm not sure what to think about DiagnosticEvent.getSource()."
It's currently used in CASSANDRA-13458 to correlate events. Ideally this would
be a session or any other transaction that you like to correlate events on.
"Gossiper - I would rather have callers ask for immutable copies, via getter
methods ... "
GossiperEvent will create an immutable copies in the ctor. According to our
style guide, getters should be avoided in favor of accessing members directly.
"You've added toString() implementations to about 10 classes."
The toString() changes should not have been part of the commit for this ticket,
but for CASSANDRA-13668 instead. Good catch. It has been implemented to provide
a short textual representation about the kind of operation that happened, as
part of an auditing event. It should be short an concise, but I don't know what
else to put there that would totally avoid breaking any potential consumers
relying on it. It's intended to be used for information purposes for users at
this point. That being said, there's no guarantees any events provided to
external consumers will never be subject to change. They will most likely do
and it's explicitly not the goal of diagnostic events to consume them for
executing control logic, but just use them for debugging and troubleshooting
instead.
"DiagnosticEvent uses millis for for the timestamp, and in particular,
System.currentTimeMillis()."
This is the event creation time, determined on best effort basis with no formal
guarantees to it whatsoever. It's really just to tell users when the event
happened.
> 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
