[
https://issues.apache.org/jira/browse/CASSANDRA-13457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16392139#comment-16392139
]
mck edited comment on CASSANDRA-13457 at 3/9/18 12:07 AM:
----------------------------------------------------------
[~spo...@gmail.com],
{quote}`diagnostic_events_enabled: true` …
[13b1e8adbd|https://github.com/apache/cassandra/commit/13b1e8adbd3f597311b7f67d71318dc939dd54fb]
{quote}
+1
{quote}`DiagnosticEvent.addIfNotNull(..)` …
[5770638d6|https://github.com/apache/cassandra/commit/5770638d689333d59f57a9ef977dbb1fa6964e30]
{quote}
+1
{quote}What's the latest trend handling this?
{quote}
I see more constructor IoC happening in places, the new streaming patches in
4.0 for example.
I guess it's mainly about avoiding the static functionality, keeping static
down to the `instance` field.
`MessagingService.instance()` does it well, in that it also does the
lazy-singleton-initialisation approach via the use of the `MSHandle` inner
static class.
I had a question about whether we can/should go further and separate the
concerns (bean vs function) in the events classes, in this
[comment|https://github.com/apache/cassandra/commit/ba09523de7cad3b3732c0e7e60b072f84e809e21#r28005488].
{quote}The {{GossiperEvent}} will access these members from the constructor to
collect relevant details for the event.
{quote}
Gotcha, I missed that, thanks.
{quote}I'd not serialize any internal classes, as we don't have any versioning
in place and classes could change quickly enough to cause deserialization
issues, even on bugfix releases.
{quote}
To clarify, I didn't mean serialising classes. I was only thinking that the
event classes could pair the `Map<String,String> toMap(..)` method with a
'MyEvent fromString(..)` as a convenience to anyone in the java space. But I
agree, it's premature and would only cause compatibility headaches.
{quote} That's also the reason I'd prefer to keep classes public, so they can
be used for unit testing.
{quote}
So long as the unit test is also in the same package the class can be
package-protected.
{quote}After some basic testing I quickly realized that way too much events
would be produced, if you happen to subscribe to them.
{quote}
You're quite right. The frequency of calls increases diagnostics, to tracing,
to metrics. What's written in that paragraph Stefan would make good dev
documentation about when and how to use each throughout the codebase.
The point that "would enable/disable repair tracing through the
{{diagnostic_events_enabled}} flag" is enough to warrant not worrying about any
attempts at pairing events at this point in time. Maybe it's part of a bigger
exercise later looking for what existing trace events should also be diagnostic
events.
was (Author: michaelsembwever):
[~spo...@gmail.com],
{quote}`diagnostic_events_enabled: true`
[13b1e8adbd|https://github.com/apache/cassandra/commit/13b1e8adbd3f597311b7f67d71318dc939dd54fb]
{quote}
+1
{quote}`DiagnosticEvent.addIfNotNull(..)`
[5770638d6|https://github.com/apache/cassandra/commit/5770638d689333d59f57a9ef977dbb1fa6964e30]
{quote}
+1
{quote}What's the latest trend handling this?
{quote}
I see more constructor IoC happening in places, the new netty streaming patches
in 4.0 for example.
I guess it's mainly about avoiding the static functionality, keeping static
down to the `instance` field.
`MessagingService.instance()` does it well, in that it also does the
lazy-singleton-initialisation approach via the use of the `MSHandle` inner
static class.
I had a question about whether we can/should go further and separate the
concerns (bean vs function) in the events classes, in this
[comment|[https://github.com/apache/cassandra/commit/ba09523de7cad3b3732c0e7e60b072f84e809e21#r28005488]|https://github.com/apache/cassandra/commit/ba09523de7cad3b3732c0e7e60b072f84e809e21#r28005488].]
{quote}The {{GossiperEvent}} will access these members from the constructor to
collect relevant details for the event.
{quote}
Gotcha, I missed that, thanks.
{quote}I'd not serialize any internal classes, as we don't have any versioning
in place and classes could change quickly enough to cause deserialization
issues, even on bugfix releases.
{quote}
To clarify, I didn't mean serialising classes. I was only thinking that the
event classes could pair the `Map<String,String> toMap(..)` method with a
'MyEvent fromString(..)` as a convenience to anyone in the java space. But I
agree, it's premature and would only cause compatibility headaches.
{quote} That's also the reason I'd prefer to keep classes public, so they can
be used for unit testing.
{quote}
So long as the unit test is also in the same package the class can be
package-protected.
{quote}After some basic testing I quickly realized that way too much events
would be produced, if you happen to subscribe to them.
{quote}
You're quite right. The frequency of calls increases diagnostics, to tracing,
to metrics. What's written in that paragraph Stefan would make good dev
documentation about when and how to use each throughout the codebase.
The point that "would enable/disable repair tracing through the
{{diagnostic_events_enabled}} flag" is enough to warrant not worrying about any
attempts at pairing events at this point in time. Maybe it's part of a bigger
exercise later looking for what existing trace events should also be diagnostic
events.
> 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
