ctubbsii commented on a change in pull request #2318:
URL: https://github.com/apache/accumulo/pull/2318#discussion_r736696806
##########
File path:
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
##########
@@ -513,7 +512,10 @@ public void run() {
Preconditions.checkState(compactionRunning.compareAndSet(false, true));
try {
LOG.info("Starting up compaction runnable for job: {}", job);
- updateCompactionState(job, TCompactionState.STARTED, "Compaction
started");
+ TCompactionStatusUpdate update = new TCompactionStatusUpdate();
+ update.setState(TCompactionState.STARTED);
+ update.setMessage("Compaction started");
+ updateCompactionState(job, update);
Review comment:
Related: a good summary of `required`, `optional`, and "default" can be
found at https://stackoverflow.com/a/37564129/196405
Everything is "optional" on read/deserialization if we're not specifying
`required` (and there's good reason to always avoid that). If we truly need a
field to be "optional" on write/serialization (mainly applicable to primitives,
since Objects can just use `null`), then yes, using the keyword `optional` will
avoid the parameter from being included in the constructor, and will require
the use of the setters to specify a value.
However, I question how often we truly need "optional" parameters. And, even
if we really do need to have an optional parameter, I question using the
`optional` keyword (instead of a `null` value) to do so. The best practices
that I've read suggest using `optional` for evolving RPC interfaces to ensure
compatibility across old and new code, and I think it can be confusing to have
it serve dual purposes by also using it for application semantics. Our thrift
interfaces are not public API, and we don't guarantee compatibility across
major/minor versions, so often, we probably don't need to use the `optional`
keyword.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
