Remove unnecessary loggings in statement handlers #2469
Conversation
KodaiD
requested review from
a team,
komamitsu,
brfrn169,
feeblefakie and
Torch3333
and removed request for
a team
| default: | ||
| logger.warn("Unsupported ordering specified. Using Order.ASC"); | ||
| return QueryBuilder.asc(quoteIfNecessary(ordering.getColumnName())); |
There was a problem hiding this comment.
I've removed the warning message for now, but I think that handling unexpected ordering values by treating them as ASC should not be performed after the operation checker's validation process. In this default branch, would it be better to throw an AssertionError instead?
There was a problem hiding this comment.
I think we should throw AssertionError in that case. Thanks.
KodaiD
changed the title
|
@brfrn169 Thanks for reviewing. It seems that OperationChecker does not validate the consistency of the Operation, and I think it's this function's responsibility to check the contents of consistency and handle unexpected cases. Therefore, I think it would be fine to leave the logging warning message here. What do you think? Ah, this line can be removed, thanks for pointing it out. I'll check if similar warning message loggings remain. |
Similar to https://github.com/scalar-labs/scalardb/pull/2469/files#r1921856962, the |
core/src/main/java/com/scalar/db/storage/cassandra/SelectStatementHandler.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/storage/cassandra/StatementHandler.java
Outdated
Show resolved
Hide resolved
Description
This PR removes unnecessary loggings in statement handlers.
In some statement handlers, the logger outputs exceptions thrown by the backend databases. However, because those exceptions are set to the cause of ExecutionException, there is no need to output them to the log.
The unnecessary error log can confuse users. For example, when a transaction gets a record with
tx_state=1, Cosmos DB throws 412(Precondition Failed), but this is an expected behavior, and there is no need to output this error to the ScalarDB log.Related issues and/or PRs
N/A
Changes made
I remove loggings in:
Other statement handlers don't do this kind of logging.
Checklist
Additional notes (optional)
N/A
Release notes
Removed unnecessary loggings in the statement handlers for Cassandra and Cosmos DB.