Skip to content

Remove unnecessary loggings in statement handlers #2469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brfrn169 merged 7 commits into from
Jan 22, 2025
Merged

Remove unnecessary loggings in statement handlers #2469

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
83fc4b4
Remove unnecessary loggings in statement handlers.
KodaiD Jan 20, 2025
5b7677c
Remove unnecessary import statements.
KodaiD Jan 20, 2025
e70d365
Remove unnecessary warning message loggings.
KodaiD Jan 21, 2025
bf353c4
Throw AssertionError instead of logging warning messages.
KodaiD Jan 21, 2025
a847075
Remove an unnecessary logger declaration.
KodaiD Jan 21, 2025
a1226d0
Update error messages.
KodaiD Jan 21, 2025
ebfb390
Merge branch 'master' into remove-unnecessary-logging
brfrn169 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/src/main/java/com/scalar/db/storage/cassandra/BatchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ public void handle(List<? extends Mutation> mutations)
CoreError.CASSANDRA_OPERATION_FAILED_IN_BATCH.buildMessage(writeType), e);
}
} catch (RuntimeException e) {
logger.warn(e.getMessage(), e);
throw new RetriableExecutionException(
CoreError.CASSANDRA_ERROR_OCCURRED_IN_BATCH.buildMessage(e.getMessage()), e);
}
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/scalar/db/storage/cassandra/MutateStatementHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public ResultSet handle(Operation operation) throws ExecutionException {
}
return results;
} catch (WriteTimeoutException e) {
logger.warn("Write timeout happened during mutate operation", e);
if (e.getWriteType() == WriteType.CAS) {
// retry needs to be done if applications need to do the operation exactly
throw new RetriableExecutionException(
Expand All @@ -71,7 +70,6 @@ public ResultSet handle(Operation operation) throws ExecutionException {
CoreError.CASSANDRA_WRITE_TIMEOUT_WITH_OTHER_WRITE_TYPE_IN_MUTATION.buildMessage(), e);
}
} catch (RuntimeException e) {
logger.warn(e.getMessage(), e);
throw new RetriableExecutionException(
CoreError.CASSANDRA_ERROR_OCCURRED_IN_MUTATION.buildMessage(e.getMessage()), e);
}
Expand Down
6 changes: 0 additions & 6 deletions core/src/main/java/com/scalar/db/storage/cassandra/SelectStatementHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
import java.util.stream.IntStream;
import javax.annotation.Nonnull;
import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A handler class for select statement
Expand All @@ -43,8 +41,6 @@
*/
@ThreadSafe
public class SelectStatementHandler extends StatementHandler {
private static final Logger logger = LoggerFactory.getLogger(SelectStatementHandler.class);

/**
* Constructs {@code SelectStatementHandler} with the specified {@code Session}
*
Expand All @@ -60,7 +56,6 @@ public ResultSet handle(Operation operation) throws ExecutionException {
try {
return handleInternal(operation);
} catch (RuntimeException e) {
logger.error(e.getMessage(), e);
throw new ExecutionException(
CoreError.CASSANDRA_ERROR_OCCURRED_IN_SELECTION.buildMessage(e.getMessage()), e);
}
Expand Down Expand Up @@ -294,7 +289,6 @@ private Ordering getOrdering(Scan.Ordering ordering) {
case DESC:
return QueryBuilder.desc(quoteIfNecessary(ordering.getColumnName()));
default:
logger.warn("Unsupported ordering specified. Using Order.ASC");
return QueryBuilder.asc(quoteIfNecessary(ordering.getColumnName()));
Copy link
Copy Markdown
Member Author

@KodaiD KodaiD Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator

@brfrn169 brfrn169 Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should throw AssertionError in that case. Thanks.

}
}
Expand Down
4 changes: 0 additions & 4 deletions core/src/main/java/com/scalar/db/storage/cosmos/BatchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import java.util.ArrayList;
import java.util.List;
import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A handler for a batch
Expand All @@ -22,7 +20,6 @@
*/
@ThreadSafe
public class BatchHandler {
private static final Logger logger = LoggerFactory.getLogger(BatchHandler.class);
private static final String MUTATION_STORED_PROCEDURE = "mutate.js";
private final CosmosClient client;
private final TableMetadataManager metadataManager;
Expand Down Expand Up @@ -88,7 +85,6 @@ private void executeStoredProcedure(
}

private void throwException(CosmosException exception) throws ExecutionException {
logger.error(exception.getMessage(), exception);
int statusCode = exception.getSubStatusCode();

if (statusCode == CosmosErrorCode.PRECONDITION_FAILED.get()) {
Expand Down
4 changes: 0 additions & 4 deletions core/src/main/java/com/scalar/db/storage/cosmos/MutateStatementHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@
import java.util.ArrayList;
import java.util.List;
import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** An abstraction for handler classes for mutate statements */
@ThreadSafe
public abstract class MutateStatementHandler extends StatementHandler {
private static final Logger logger = LoggerFactory.getLogger(MutateStatementHandler.class);
private static final String MUTATION_STORED_PROCEDURE = "mutate.js";

public MutateStatementHandler(CosmosClient client, TableMetadataManager metadataManager) {
Expand Down Expand Up @@ -60,7 +57,6 @@ protected void executeStoredProcedure(Mutation mutation, TableMetadata tableMeta
}

private void throwException(CosmosException exception) throws ExecutionException {
logger.error(exception.getMessage());
int statusCode = exception.getSubStatusCode();

if (statusCode == CosmosErrorCode.PRECONDITION_FAILED.get()) {
Expand Down