Skip to content

Commit

Permalink
Update spool to flush on size thresholds instead of batch counts (#930)
Browse files Browse the repository at this point in the history
  • Loading branch information
mlw authored Nov 3, 2022
1 parent 8c531a2 commit 1adb6d2
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 13 deletions.
4 changes: 2 additions & 2 deletions Source/common/SNTConfigurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@
///
/// If eventLogType is set to protobuf, spoolDirectoryFileSizeThresholdKB sets the per-file size
/// limit for files saved in the spoolDirectory.
/// Defaults to 100.
/// Defaults to 250.
///
/// @note: This property is KVO compliant, but should only be read once at santad startup.
///
Expand All @@ -232,7 +232,7 @@
///
/// If eventLogType is set to protobuf, spoolDirectoryEventMaxFlushTimeSec sets the maximum amount
/// of time an event will be stored in memory before being written to disk.
/// Defaults to 10.0.
/// Defaults to 15.0.
///
/// @note: This property is KVO compliant, but should only be read once at santad startup.
///
Expand Down
4 changes: 2 additions & 2 deletions Source/common/SNTConfigurator.m
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ - (NSString *)spoolDirectory {
- (NSUInteger)spoolDirectoryFileSizeThresholdKB {
return self.configState[kSpoolDirectoryFileSizeThresholdKB]
? [self.configState[kSpoolDirectoryFileSizeThresholdKB] unsignedIntegerValue]
: 100;
: 250;
}

- (NSUInteger)spoolDirectorySizeThresholdMB {
Expand All @@ -782,7 +782,7 @@ - (NSUInteger)spoolDirectorySizeThresholdMB {
- (float)spoolDirectoryEventMaxFlushTimeSec {
return self.configState[kSpoolDirectoryEventMaxFlushTimeSec]
? [self.configState[kSpoolDirectoryEventMaxFlushTimeSec] floatValue]
: 10.0;
: 15.0;
}

- (BOOL)enableMachineIDDecoration {
Expand Down
7 changes: 5 additions & 2 deletions Source/santad/Logs/EndpointSecurity/Writers/Spool.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ class Spool : public Writer, public std::enable_shared_from_this<Spool> {
public:
// Factory
static std::shared_ptr<Spool> Create(std::string_view base_dir, size_t max_spool_disk_size,
size_t max_spool_batch_size, uint64_t flush_timeout_ms);
size_t max_spool_file_size, uint64_t flush_timeout_ms);

Spool(dispatch_queue_t q, dispatch_source_t timer_source, std::string_view base_dir,
size_t max_spool_disk_size, size_t max_spool_batch_size,
size_t max_spool_disk_size, size_t max_spool_file_size,
void (^write_complete_f)(void) = nullptr, void (^flush_task_complete_f)(void) = nullptr);

~Spool();
Expand All @@ -58,10 +58,13 @@ class Spool : public Writer, public std::enable_shared_from_this<Spool> {
dispatch_source_t timer_source_ = NULL;
::fsspool::FsSpoolWriter spool_writer_;
::fsspool::FsSpoolLogBatchWriter log_batch_writer_;
size_t spool_file_size_threshold_;
std::string type_url_;
bool flush_task_started_ = false;
void (^write_complete_f_)(void);
void (^flush_task_complete_f_)(void);

size_t accumulated_bytes_ = 0;
};

} // namespace santa::santad::logs::endpoint_security::writers
Expand Down
30 changes: 23 additions & 7 deletions Source/santad/Logs/EndpointSecurity/Writers/Spool.mm
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,19 @@
return spool_writer;
}

// Note: The `log_batch_writer_` has the batch size set to SIZE_T_MAX. This is because
// the decision on whether or not to flush is controlled by the Spool class here based
// on a "size of bytes" threshold, not "count of records" threshold used by the
// FsSpoolLogBatchWriter. As such, calling `FsSpoolLogBatchWriter::WriteMessage`
// should never flush.
Spool::Spool(dispatch_queue_t q, dispatch_source_t timer_source, std::string_view base_dir,
size_t max_spool_disk_size, size_t max_spool_batch_size,
void (^write_complete_f)(void), void (^flush_task_complete_f)(void))
size_t max_spool_disk_size, size_t max_spool_file_size, void (^write_complete_f)(void),
void (^flush_task_complete_f)(void))
: q_(q),
timer_source_(timer_source),
spool_writer_(absl::string_view(base_dir.data(), base_dir.length()), max_spool_disk_size),
log_batch_writer_(&spool_writer_, max_spool_batch_size),
log_batch_writer_(&spool_writer_, SIZE_T_MAX),
spool_file_size_threshold_(max_spool_file_size),
write_complete_f_(write_complete_f),
flush_task_complete_f_(flush_task_complete_f) {
type_url_ = kTypeGoogleApisComPrefix + ::santa::pb::v1::SantaMessage::descriptor()->full_name();
Expand Down Expand Up @@ -87,7 +93,12 @@
}

bool Spool::Flush() {
return log_batch_writer_.Flush().ok();
if (log_batch_writer_.Flush().ok()) {
accumulated_bytes_ = 0;
return true;
} else {
return false;
}
}

void Spool::Write(std::vector<uint8_t> &&bytes) {
Expand All @@ -108,13 +119,18 @@
#endif
any.set_type_url(type_url_);

auto status = log_batch_writer_.WriteMessage(any);
auto status = shared_this->log_batch_writer_.WriteMessage(any);
if (!status.ok()) {
LOGE(@"ProtoEventLogger::LogProto failed with: %s", status.ToString().c_str());
}

if (write_complete_f_) {
write_complete_f_();
shared_this->accumulated_bytes_ += moved_bytes.size();
if (shared_this->accumulated_bytes_ >= shared_this->spool_file_size_threshold_) {
shared_this->Flush();
}

if (shared_this->write_complete_f_) {
shared_this->write_complete_f_();
}
});
}
Expand Down

0 comments on commit 1adb6d2

Please sign in to comment.