fix(journal-index): avoid ValueGuardInUse when indexing otel journals (#22621)
* fix(journal-index): avoid ValueGuardInUse when indexing otel journals collect_remapping_entry_offsets() held a ValueGuard<DataObject> (which keeps the journal file's window-manager borrow alive) while calling InlinedCursor::collect_offsets(). When the ND_REMAPPING=1 data object is referenced by more than one entry, collect_offsets() walks the entry-array chain, re-borrowing the window manager and failing with JournalError::ValueGuardInUse ("previous object is still in use"). This aborted indexing of every otel-plugin journal (which always contains an ND_REMAPPING bookkeeping record), so the otel-logs viewer returned "no data". Regular systemd journals lack the field, so the loop never ran and they were unaffected. Split the work into two phases, mirroring collect_source_field_info(): first copy out each data object's InlinedCursor (a Copy value) so the data-object guard drops, then walk the cursors' entry-array chains once no guard is held. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(journal-index): regression test for ND_REMAPPING multi-entry indexing Builds a journal with several ND_REMAPPING=1 bookkeeping entries (which share one multi-entry DATA object) plus normal log entries, and asserts FileIndexer::index() succeeds. Before the guard fix this fails with JournalError::ValueGuardInUse: collect_remapping_entry_offsets walks the marker object's entry-array chain while still holding the DataObject window guard. Verified the test fails on the pre-fix code and passes after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vkalintiris committed
Jun 3, 2026 at 17:23 UTC
5ac1358c4213dcba403efe384e7dcde2da41c2a3
2 files changed
+122
src/crates/journal-index/src/file_indexer.rs
+19
@@ -473,6 +473,19 @@ impl FileIndexer {
473
// silently flow into the index.
474
let field_data_iterator = journal_file.field_data_objects(marker_field)?;
475
476
+ // Phase 1: collect the inlined cursor of every ND_REMAPPING=1 data
477
+ // object.
478
+ //
479
+ // Each iterator item is a `ValueGuard<DataObject>` that holds the
480
+ // journal file's window-manager borrow for as long as it is alive.
481
+ // `InlinedCursor` is a small `Copy` value, so we copy it out and let
482
+ // the data-object guard drop at the end of each iteration. We must
483
+ // NOT call `collect_offsets()` here: when a data object is referenced
484
+ // by more than one entry it walks the entry-array chain, which
485
+ // re-borrows the window manager. Doing that while a data-object guard
486
+ // is still live fails with `ValueGuardInUse`. This mirrors the
487
+ // two-phase approach in `collect_source_field_info`.
488
+ let mut remapping_cursors = Vec::new();
489
for data_object in field_data_iterator {
490
let data_object = data_object?;
491
@@ -485,6 +498,12 @@ impl FileIndexer {
498
continue;
499
};
500
501
+ remapping_cursors.push(inlined_cursor);
502
+ }
503
+
504
+ // Phase 2: walk each cursor's entry-array chain. No data-object guard
505
+ // is held at this point, so re-borrowing the window manager is safe.
506
+ for inlined_cursor in remapping_cursors {
507
self.entry_offsets.clear();
508
inlined_cursor.collect_offsets(journal_file, &mut self.entry_offsets)?;
509
src/crates/journal-index/tests/remapping_indexing.rs
new
+103
@@ -0,0 +1,103 @@
1
+//! Regression test for indexing journals that contain the otel-plugin's
2
+//! `ND_REMAPPING=1` bookkeeping field.
3
+//!
4
+//! `FileIndexer::collect_remapping_entry_offsets` used to hold a
5
+//! `ValueGuard<DataObject>` (which keeps the journal file's window-manager
6
+//! borrow alive) while calling `InlinedCursor::collect_offsets`. When the
7
+//! `ND_REMAPPING=1` data object is referenced by more than one entry,
8
+//! `collect_offsets` walks the entry-array chain, re-borrows the window
9
+//! manager, and fails with `JournalError::ValueGuardInUse` ("previous object
10
+//! is still in use"). That aborted indexing of every otel-plugin journal.
11
+//!
12
+//! Regular systemd journals do not contain the `ND_REMAPPING` field, so the
13
+//! existing tests never exercised this path. This test reproduces the
14
+//! multi-entry shape that triggered the bug and asserts indexing succeeds.
15
+
16
+use journal_common::Seconds;
17
+use journal_core::field_map::REMAPPING_MARKER;
18
+use journal_core::file::{JournalFile, JournalFileOptions, JournalWriter};
19
+use journal_core::repository::File;
20
+use journal_index::{FieldName, FileIndexer};
21
+use std::fs;
22
+use std::path::PathBuf;
23
+use tempfile::TempDir;
24
+use uuid::Uuid;
25
+
26
+fn create_test_journal_path(temp_dir: &TempDir) -> PathBuf {
27
+ let machine_id = Uuid::from_u128(0x12345678_1234_1234_1234_123456789abc);
28
+ let machine_dir = temp_dir.path().join(machine_id.to_string());
29
+ fs::create_dir_all(&machine_dir).expect("create machine dir");
30
+ machine_dir.join("system.journal")
31
+}
32
+
33
+/// Build a journal that mirrors an otel-plugin journal: a handful of
34
+/// `ND_REMAPPING=1` bookkeeping entries plus normal log entries.
35
+///
36
+/// The bookkeeping entries all share one `ND_REMAPPING=1` DATA object (journald
37
+/// dedups by payload), so with `num_marker_entries >= 2` that object's
38
+/// entry-array chain is non-empty — the shape that made
39
+/// `collect_remapping_entry_offsets` re-borrow the window manager and fail.
40
+///
41
+/// The normal entries (which carry no marker) are excluded neither from the
42
+/// histogram nor the bitmaps, so a fixed indexer produces a non-empty index
43
+/// instead of erroring with `EmptyHistogramInput`.
44
+fn create_remapping_journal(num_marker_entries: u64, num_real_entries: u64) -> (TempDir, File) {
45
+ let temp_dir = TempDir::new().expect("temp dir");
46
+ let journal_path = create_test_journal_path(&temp_dir);
47
+ let file = File::from_path(&journal_path).expect("File::from_path");
48
+
49
+ let machine_id = Uuid::from_u128(0x12345678_1234_1234_1234_123456789abc);
50
+ let boot_id = Uuid::from_u128(0x11111111_1111_1111_1111_111111111111);
51
+ let seqnum_id = Uuid::from_u128(0x22222222_2222_2222_2222_222222222222);
52
+
53
+ let options = JournalFileOptions::new(machine_id, boot_id, seqnum_id);
54
+ let mut journal_file = JournalFile::create(&file, options).expect("create journal");
55
+ let mut writer = JournalWriter::new(&mut journal_file, 1, boot_id).expect("writer");
56
+
57
+ let mut timestamp = 1_000_000u64; // microseconds, strictly increasing
58
+
59
+ // Bookkeeping entries: timestamp + the shared ND_REMAPPING=1 marker.
60
+ for _ in 0..num_marker_entries {
61
+ timestamp += 1;
62
+ let ts_field = format!("_SOURCE_REALTIME_TIMESTAMP={timestamp}").into_bytes();
63
+ let items: Vec<&[u8]> = vec![ts_field.as_slice(), REMAPPING_MARKER];
64
+ writer
65
+ .add_entry(&mut journal_file, &items, timestamp, timestamp)
66
+ .expect("add marker entry");
67
+ }
68
+
69
+ // Real log entries: no marker, so they survive into the index.
70
+ for i in 0..num_real_entries {
71
+ timestamp += 1;
72
+ let ts_field = format!("_SOURCE_REALTIME_TIMESTAMP={timestamp}").into_bytes();
73
+ let message = format!("MESSAGE=log entry {i}").into_bytes();
74
+ let items: Vec<&[u8]> = vec![ts_field.as_slice(), message.as_slice()];
75
+ writer
76
+ .add_entry(&mut journal_file, &items, timestamp, timestamp)
77
+ .expect("add real entry");
78
+ }
79
+
80
+ (temp_dir, file)
81
+}
82
+
83
+/// Indexing a journal whose `ND_REMAPPING=1` object spans multiple entries
84
+/// must not fail with `ValueGuardInUse`.
85
+#[test]
86
+fn index_journal_with_multi_entry_remapping_marker() {
87
+ // Two marker entries already produce an entry-array chain (one inlined
88
+ // offset + one array object), the minimal shape that triggered the bug;
89
+ // use a few more to be robust against array-layout changes.
90
+ let (_temp_dir, file) = create_remapping_journal(3, 5);
91
+
92
+ let mut indexer = FileIndexer::default();
93
+ let source = FieldName::new("_SOURCE_REALTIME_TIMESTAMP").unwrap();
94
+ let message = FieldName::new("MESSAGE").unwrap();
95
+
96
+ let result = indexer.index(&file, Some(&source), &[message], Seconds(15));
97
+
98
+ assert!(
99
+ result.is_ok(),
100
+ "indexing a journal with a multi-entry ND_REMAPPING marker must succeed, got: {:?}",
101
+ result.err()
102
+ );
103
+}