Skip to content

Commit ecaa650

Browse files
netudimasmiklosovic
authored andcommitted
Optimize audit logic for batch operations especially when audit is not enabled for DML
Avoid audit batch event creation if audit for batch is not enabled (random UIID generation is not very cheap) Avoid String.format in a potential hot path Patch by Dmitry Konstantinov; reviewed by Štefan Miklošovič for CASSANDRA-20885 Co-authored-by: Štefan Miklošovič
1 parent 39302dd commit ecaa650

File tree

3 files changed

+51
-37
lines changed

3 files changed

+51
-37
lines changed

CHANGES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
5.1
2+
* Optimize audit logic for batch operations especially when audit is not enabled for DML (CASSANDRA-20885)
23
* Implement nodetool history (CASSANDRA-20851)
34
* Expose StorageService.dropPreparedStatements via JMX (CASSANDRA-20870)
45
* Expose Metric for Prepared Statement Cache Size (in bytes) (CASSANDRA-20864)

src/java/org/apache/cassandra/audit/AuditLogFilter.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,17 @@ boolean isFiltered(AuditLogEntry auditLogEntry)
153153
|| isFiltered(auditLogEntry.getUser(), includedUsers, excludedUsers);
154154
}
155155

156+
boolean isFiltered(AuditLogContext auditLogContext)
157+
{
158+
return isFiltered(auditLogContext.keyspace, includedKeyspaces, excludedKeyspaces)
159+
|| isFiltered(auditLogContext.auditLogEntryType.getCategory().toString(), includedCategories, excludedCategories);
160+
}
161+
162+
boolean isFiltered(AuditLogEntryType auditLogEntryType)
163+
{
164+
return isFiltered(auditLogEntryType.getCategory().toString(), includedCategories, excludedCategories);
165+
}
166+
156167
/**
157168
* Checks whether given input is being filtered or not.
158169
* If excludeSet does not contain any items, by default nothing is excluded (unless there are

src/java/org/apache/cassandra/audit/AuditLogManager.java

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.security.AccessControlContext;
2727
import java.security.AccessController;
2828
import java.security.Principal;
29-
import java.util.ArrayList;
3029
import java.util.Collections;
3130
import java.util.List;
3231
import java.util.Objects;
@@ -315,12 +314,47 @@ else if (statement != null)
315314
log(entry, cause, query == null ? null : ImmutableList.of(query));
316315
}
317316

318-
public void batchSuccess(BatchStatement.Type batchType, List<? extends CQLStatement> statements, List<String> queries, List<List<ByteBuffer>> values, QueryOptions options, QueryState state, long queryTime, Message.Response response)
317+
public void batchSuccess(BatchStatement.Type batchType,
318+
List<? extends CQLStatement> statements,
319+
List<String> queries,
320+
List<List<ByteBuffer>> values,
321+
QueryOptions options,
322+
QueryState state,
323+
long queryStartTimeMillis,
324+
Message.Response response)
319325
{
320-
List<AuditLogEntry> entries = buildEntriesForBatch(statements, queries, state, options, queryTime);
321-
for (AuditLogEntry auditLogEntry : entries)
326+
UUID batchId = null;
327+
if (!filter.isFiltered(AuditLogEntryType.BATCH))
322328
{
323-
log(auditLogEntry);
329+
batchId = UUID.randomUUID(); // lazy init only if needed, to reduce overheads
330+
log(new AuditLogEntry.Builder(state)
331+
.setOperation("BatchId:[" + batchId + "] - BATCH of [" + statements.size() + "] statements")
332+
.setOptions(options)
333+
.setTimestamp(queryStartTimeMillis)
334+
.setBatch(batchId)
335+
.setType(AuditLogEntryType.BATCH)
336+
.build());
337+
}
338+
339+
for (int i = 0; i < statements.size(); i++)
340+
{
341+
CQLStatement statement = statements.get(i);
342+
if (filter.isFiltered(statement.getAuditLogContext()))
343+
continue;
344+
345+
if (batchId == null)
346+
batchId = UUID.randomUUID();
347+
348+
log(new AuditLogEntry.Builder(state)
349+
.setType(statement.getAuditLogContext().auditLogEntryType)
350+
.setOperation(queries.get(i))
351+
.setTimestamp(queryStartTimeMillis)
352+
.setScope(statement)
353+
.setKeyspace(state, statement)
354+
.setOptions(options)
355+
.setBatch(batchId)
356+
.build());
357+
324358
}
325359
}
326360

@@ -334,38 +368,6 @@ public void batchFailure(BatchStatement.Type batchType, List<? extends CQLStatem
334368
log(entry, cause, queries);
335369
}
336370

337-
private static List<AuditLogEntry> buildEntriesForBatch(List<? extends CQLStatement> statements, List<String> queries, QueryState state, QueryOptions options, long queryStartTimeMillis)
338-
{
339-
List<AuditLogEntry> auditLogEntries = new ArrayList<>(statements.size() + 1);
340-
UUID batchId = UUID.randomUUID();
341-
String queryString = String.format("BatchId:[%s] - BATCH of [%d] statements", batchId, statements.size());
342-
AuditLogEntry entry = new AuditLogEntry.Builder(state)
343-
.setOperation(queryString)
344-
.setOptions(options)
345-
.setTimestamp(queryStartTimeMillis)
346-
.setBatch(batchId)
347-
.setType(AuditLogEntryType.BATCH)
348-
.build();
349-
auditLogEntries.add(entry);
350-
351-
for (int i = 0; i < statements.size(); i++)
352-
{
353-
CQLStatement statement = statements.get(i);
354-
entry = new AuditLogEntry.Builder(state)
355-
.setType(statement.getAuditLogContext().auditLogEntryType)
356-
.setOperation(queries.get(i))
357-
.setTimestamp(queryStartTimeMillis)
358-
.setScope(statement)
359-
.setKeyspace(state, statement)
360-
.setOptions(options)
361-
.setBatch(batchId)
362-
.build();
363-
auditLogEntries.add(entry);
364-
}
365-
366-
return auditLogEntries;
367-
}
368-
369371
public void prepareSuccess(CQLStatement statement, String query, QueryState state, long queryTime, ResultMessage.Prepared response)
370372
{
371373
AuditLogEntry entry = new AuditLogEntry.Builder(state).setOperation(query)

0 commit comments

Comments
 (0)