[WIP] 2.0 preview #281
[WIP] 2.0 preview #281
Conversation
Guardian actors Reporter and module actor Subscriptions and message types Initial patching
| } | ||
|
|
||
| /** | ||
| * * Create actor props from class type without {@code Configuration} parameter. |
There was a problem hiding this comment.
How without config param when it is second argument in method?
| /** | ||
| * Measurement tags. | ||
| */ | ||
| public final Map<String, String> tags; |
There was a problem hiding this comment.
Shouldn't it be better to have Map<String, Object> tags instead of Map<String, String>?
There was a problem hiding this comment.
We discussed about this. It does make sense for it to be <String, Object> but these changes should be implemented after the switch to actors.
| /** | ||
| * Measurement fields. | ||
| */ | ||
| public final Map<String, String> fields; |
| * @return JSON-formatted string representation of measurement. | ||
| */ | ||
| public String toJson() { | ||
| return "{\"name\":\"" + name + "\"" + ",\"type\":\"" + type + "\"" + ",\"value\":" + value + ",\"time\":" + time |
There was a problem hiding this comment.
Why not use Jackson or some other library to convert value to JSON instead of coding it?
| (reporter) -> mediator.tell(new DistributedPubSubMediator.Publish(reporter, measurement), getSelf())); | ||
| } | ||
|
|
||
| private void terminate() { |
There was a problem hiding this comment.
Why is terminate private and other methods are protected (stop, start, process) ?
There was a problem hiding this comment.
This is an internal call for ModuleActor termination and there is no need for it to be public. The function itself is missing and will be added.
| /** | ||
| * Connector implementation. | ||
| */ | ||
| public class ConnectorProxyImpl implements ConnectorProxy { |
There was a problem hiding this comment.
It might be personal, but I don't like implementation classes having *Impl sufix. This does not explain anything specific to this concrete implementation. If there is nothing specific for this implementation, it is better to call it DefaultConnectorProxy or BasicConnectorProxy
There was a problem hiding this comment.
Personally I prefer IConnectorProxy naming of interfaces where the default implementation would then be ConnectorProxy.
| .named("org.apache.cassandra.cql3.QueryProcessor"); | ||
|
|
||
| // Transformer for QueryProcessor | ||
| final ByteBuddy byteBuddy = new ByteBuddy().with(Implementation.Context.Disabled.Factory.INSTANCE); |
There was a problem hiding this comment.
What is Implementation.Context.Disabled.Factory.INSTANCE ?
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ConnectorProxyImpl.class); | ||
|
|
||
| private static QueryProcessorWrapper queryProcessorWrapper; |
There was a problem hiding this comment.
Because of funky bytebuddy initialization. I'll add an issue to switch to a much newer version of bytebuddy when this part of the work is done.
| /** | ||
| * Connector proxy implementation. | ||
| */ | ||
| public class ConnectorProxyImpl implements ConnectorProxy { |
There was a problem hiding this comment.
Same comment for naming as for ConnectorProxyImpl within cassandra 2.1
| initEndpoints(); | ||
| nodeGuardian.tell(new Command.Start(), null); | ||
|
|
||
| // this.diagnosticsProcessor = new DiagnosticsProcessor(config); |
There was a problem hiding this comment.
Why is this commented out? If it is not needed, we should remove it.
Changes towards 2.0.0 version
Switching to actors and decoupling of components