Skip to content

Commit 1b3af17

Browse files
tot19claude
andcommitted
fix(sources): Fix acknowledgment checkpoint race in Windows Event Log source
The background checkpoint saver task was updating checkpoints every 5 seconds regardless of acknowledgment status, bypassing the end-to-end acknowledgment system. This caused data loss when Vector was killed because events could be checkpointed before the sink confirmed delivery. Changes: - Disable background checkpoint saver when acknowledgements are enabled - Checkpoints are now managed solely by the Finalizer when acks are enabled - Update changelog to document acknowledgment support and wildcard limitation When acks are disabled, the background checkpoint saver continues to operate as before for performance (checkpoint every 5 seconds). When acks are enabled, checkpoints only update after sink confirmation, ensuring at-least-once delivery. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 58d513d commit 1b3af17

3 files changed

Lines changed: 93 additions & 74 deletions

File tree

‎changelog.d/windows_eventlog_source.feature.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ This Windows-specific source uses event-driven subscriptions to stream events in
66
- Real-time event-driven subscriptions using EvtSubscribe API
77
- XPath query filtering for selective event collection
88
- Checkpoint persistence for reliable resumption after restarts (similar to journald)
9+
- End-to-end acknowledgment support for at-least-once delivery guarantees
910
- Configurable rate limiting to prevent overwhelming downstream systems
1011
- Configurable field truncation for storage optimization (Winlogbeat-compatible)
1112
- Enhanced provider name extraction using proper XML parsing
@@ -18,3 +19,5 @@ New configuration options:
1819
- `events_per_second`: Rate limit event processing (0 = unlimited)
1920
- `max_event_data_length`: Truncate event data values (0 = no truncation, Winlogbeat default)
2021
- `data_dir`: Directory for checkpoint persistence
22+
23+
Note: Wildcard channel patterns (e.g., `Microsoft-Windows-*`) are not supported. Specify exact channel names.

‎src/sources/windows_eventlog/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ impl WindowsEventLogSource {
178178
);
179179

180180
let mut subscription =
181-
EventLogSubscription::new(&self.config, Arc::clone(&checkpointer)).await?;
181+
EventLogSubscription::new(&self.config, Arc::clone(&checkpointer), self.acknowledgements).await?;
182182
let parser = EventLogParser::new(&self.config);
183183

184184
let events_received = register!(EventsReceived);

‎src/sources/windows_eventlog/subscription.rs‎

Lines changed: 89 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,17 @@ impl CallbackContext {
210210

211211
impl EventLogSubscription {
212212
/// Create a new event-driven subscription using EvtSubscribe with callback
213+
///
214+
/// # Arguments
215+
/// * `config` - Configuration for the Windows Event Log source
216+
/// * `checkpointer` - Shared checkpointer for persisting bookmarks
217+
/// * `acknowledgements` - Whether end-to-end acknowledgements are enabled.
218+
/// When true, the background checkpoint saver is disabled because checkpoints
219+
/// are managed by the finalizer after sink acknowledgement.
213220
pub async fn new(
214221
config: &WindowsEventLogConfig,
215222
checkpointer: Arc<Checkpointer>,
223+
acknowledgements: bool,
216224
) -> Result<Self, WindowsEventLogError> {
217225
// Create rate limiter if configured
218226
let rate_limiter = if config.events_per_second > 0 {
@@ -290,75 +298,83 @@ impl EventLogSubscription {
290298

291299
// Start background task to periodically save bookmarks (every 5 seconds)
292300
// This avoids blocking event processing with serialization/I/O
293-
let checkpoint_saver_ctx = Arc::clone(&callback_context);
294-
let mut shutdown_rx_clone = shutdown_rx.clone();
295-
tokio::spawn(async move {
296-
debug!(message = "Checkpoint saver background task started");
297-
let mut interval = tokio::time::interval(tokio::time::Duration::from_secs(5));
298-
loop {
299-
// Wait for either interval tick or shutdown signal
300-
tokio::select! {
301-
_ = interval.tick() => {}
302-
_ = shutdown_rx_clone.changed() => {
303-
debug!(message = "Checkpoint saver task received shutdown signal");
304-
break;
301+
//
302+
// IMPORTANT: Only run this when acknowledgements are DISABLED.
303+
// When acks are enabled, checkpoints are managed by the Finalizer
304+
// which only updates after the sink confirms delivery.
305+
if !acknowledgements {
306+
let checkpoint_saver_ctx = Arc::clone(&callback_context);
307+
let mut shutdown_rx_clone = shutdown_rx.clone();
308+
tokio::spawn(async move {
309+
debug!(message = "Checkpoint saver background task started (acks disabled)");
310+
let mut interval = tokio::time::interval(tokio::time::Duration::from_secs(5));
311+
loop {
312+
// Wait for either interval tick or shutdown signal
313+
tokio::select! {
314+
_ = interval.tick() => {}
315+
_ = shutdown_rx_clone.changed() => {
316+
debug!(message = "Checkpoint saver task received shutdown signal");
317+
break;
318+
}
305319
}
306-
}
307320

308-
// Step 1: Copy handles while holding lock briefly (handles are just integers)
309-
let bookmark_handles: Vec<(
310-
String,
311-
windows::Win32::System::EventLog::EVT_HANDLE,
312-
)> = {
313-
let bookmarks = match checkpoint_saver_ctx.bookmarks.read() {
314-
Ok(guard) => guard,
315-
Err(poisoned) => {
316-
warn!(message = "Bookmark lock poisoned, recovering");
317-
poisoned.into_inner()
318-
}
319-
};
320-
bookmarks
321-
.iter()
322-
.map(|(channel, bookmark)| (channel.clone(), bookmark.as_handle()))
323-
.collect()
324-
}; // Lock released IMMEDIATELY
325-
326-
// Step 2: Serialize handles OUTSIDE the lock (slow Windows API calls)
327-
let bookmark_xmls: Vec<(String, String)> = bookmark_handles
328-
.into_iter()
329-
.filter_map(|(channel, handle)| {
330-
match BookmarkManager::serialize_handle(handle) {
331-
Ok(xml) if Self::is_valid_bookmark_xml(&xml) => {
332-
Some((channel, xml))
321+
// Step 1: Copy handles while holding lock briefly (handles are just integers)
322+
let bookmark_handles: Vec<(
323+
String,
324+
windows::Win32::System::EventLog::EVT_HANDLE,
325+
)> = {
326+
let bookmarks = match checkpoint_saver_ctx.bookmarks.read() {
327+
Ok(guard) => guard,
328+
Err(poisoned) => {
329+
warn!(message = "Bookmark lock poisoned, recovering");
330+
poisoned.into_inner()
333331
}
334-
Ok(_) => None, // Empty or invalid = bookmark not yet updated with events
335-
Err(e) => {
336-
emit!(WindowsEventLogBookmarkError {
337-
channel: channel.clone(),
338-
error: e.to_string(),
339-
});
340-
None
332+
};
333+
bookmarks
334+
.iter()
335+
.map(|(channel, bookmark)| (channel.clone(), bookmark.as_handle()))
336+
.collect()
337+
}; // Lock released IMMEDIATELY
338+
339+
// Step 2: Serialize handles OUTSIDE the lock (slow Windows API calls)
340+
let bookmark_xmls: Vec<(String, String)> = bookmark_handles
341+
.into_iter()
342+
.filter_map(|(channel, handle)| {
343+
match BookmarkManager::serialize_handle(handle) {
344+
Ok(xml) if Self::is_valid_bookmark_xml(&xml) => {
345+
Some((channel, xml))
346+
}
347+
Ok(_) => None, // Empty or invalid = bookmark not yet updated with events
348+
Err(e) => {
349+
emit!(WindowsEventLogBookmarkError {
350+
channel: channel.clone(),
351+
error: e.to_string(),
352+
});
353+
None
354+
}
341355
}
356+
})
357+
.collect();
358+
359+
// Save all checkpoints in a single batched disk write (no locks held)
360+
if !bookmark_xmls.is_empty() {
361+
if let Err(e) = checkpoint_saver_ctx
362+
.checkpointer()
363+
.set_batch(bookmark_xmls)
364+
.await
365+
{
366+
error!(
367+
message = "Failed to save bookmark checkpoints",
368+
error = %e
369+
);
342370
}
343-
})
344-
.collect();
345-
346-
// Save all checkpoints in a single batched disk write (no locks held)
347-
if !bookmark_xmls.is_empty() {
348-
if let Err(e) = checkpoint_saver_ctx
349-
.checkpointer()
350-
.set_batch(bookmark_xmls)
351-
.await
352-
{
353-
error!(
354-
message = "Failed to save bookmark checkpoints",
355-
error = %e
356-
);
357371
}
358372
}
359-
}
360-
debug!(message = "Checkpoint saver task terminated");
361-
});
373+
debug!(message = "Checkpoint saver task terminated");
374+
});
375+
} else {
376+
debug!(message = "Background checkpoint saver disabled (acks enabled)");
377+
}
362378

363379
Ok(Self {
364380
config,
@@ -1641,7 +1657,7 @@ mod tests {
16411657
async fn test_not_supported_error() {
16421658
let config = WindowsEventLogConfig::default();
16431659
let (checkpointer, _temp_dir) = create_test_checkpointer().await;
1644-
let result = EventLogSubscription::new(&config, checkpointer).await;
1660+
let result = EventLogSubscription::new(&config, checkpointer, false).await;
16451661

16461662
assert!(matches!(
16471663
result,
@@ -1674,7 +1690,7 @@ mod tests {
16741690

16751691
let (checkpointer, _temp_dir) = create_test_checkpointer().await;
16761692

1677-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
1693+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
16781694
.await
16791695
.expect("Subscription creation should succeed");
16801696

@@ -1824,7 +1840,7 @@ mod tests {
18241840
let (checkpointer, _temp_dir) = create_test_checkpointer().await;
18251841

18261842
// Create subscription - should succeed initially as EvtSubscribe doesn't validate query syntax
1827-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
1843+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
18281844
.await
18291845
.expect("Subscription creation should succeed");
18301846

@@ -1866,7 +1882,7 @@ mod tests {
18661882

18671883
let (checkpointer, _temp_dir) = create_test_checkpointer().await;
18681884

1869-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
1885+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
18701886
.await
18711887
.expect("Subscription creation should succeed");
18721888

@@ -1899,7 +1915,7 @@ mod tests {
18991915

19001916
let (checkpointer, _temp_dir) = create_test_checkpointer().await;
19011917

1902-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
1918+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
19031919
.await
19041920
.expect("Subscription creation should succeed");
19051921

@@ -1943,13 +1959,13 @@ mod tests {
19431959
let (checkpointer3, _temp_dir3) = create_test_checkpointer().await;
19441960

19451961
// Create all three subscriptions concurrently
1946-
let mut sub1 = EventLogSubscription::new(&config1, checkpointer1)
1962+
let mut sub1 = EventLogSubscription::new(&config1, checkpointer1, false)
19471963
.await
19481964
.expect("Subscription 1 (Application) should succeed");
1949-
let mut sub2 = EventLogSubscription::new(&config2, checkpointer2)
1965+
let mut sub2 = EventLogSubscription::new(&config2, checkpointer2, false)
19501966
.await
19511967
.expect("Subscription 2 (System) should succeed");
1952-
let mut sub3 = EventLogSubscription::new(&config3, checkpointer3)
1968+
let mut sub3 = EventLogSubscription::new(&config3, checkpointer3, false)
19531969
.await
19541970
.expect("Subscription 3 (Security) should succeed");
19551971

@@ -2026,7 +2042,7 @@ mod tests {
20262042
// Record the time BEFORE creating the subscription
20272043
let subscription_start_time = Utc::now();
20282044

2029-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
2045+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
20302046
.await
20312047
.expect("Subscription creation should succeed");
20322048

@@ -2085,7 +2101,7 @@ mod tests {
20852101

20862102
let subscription_start_time = Utc::now();
20872103

2088-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
2104+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
20892105
.await
20902106
.expect("Subscription creation should succeed");
20912107

@@ -2145,7 +2161,7 @@ mod tests {
21452161
config.read_existing_events = true; // Even with this true, checkpoint should take precedence
21462162
config.event_timeout_ms = 500;
21472163

2148-
let mut subscription = EventLogSubscription::new(&config, checkpointer)
2164+
let mut subscription = EventLogSubscription::new(&config, checkpointer, false)
21492165
.await
21502166
.expect("Subscription creation should succeed");
21512167

0 commit comments

Comments
 (0)