Skip to content

Commit bfb01ae

Browse files
committed
perf(server): optimize writing performance
This commit optimizes the log writing process by removing the stream_position() call, which uses lseek and negatively impacts performance. Afterwards, the new implementation directly updates the log file size using fetch_add, resulting in a 40% performance improvement on writing (Linux). On MacOS, degradation was not visible.
1 parent 0d9ca75 commit bfb01ae

File tree

4 files changed

+30
-48
lines changed

4 files changed

+30
-48
lines changed

Cargo.lock

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "server"
3-
version = "0.4.200"
3+
version = "0.4.201"
44
edition = "2021"
55
build = "src/build.rs"
66
license = "Apache-2.0"

server/src/streaming/segments/logs/log_writer.rs

+6-16
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use std::{
1414
};
1515
use tokio::{
1616
fs::{File, OpenOptions},
17-
io::{AsyncSeekExt, AsyncWriteExt},
17+
io::AsyncWriteExt,
1818
sync::RwLock,
1919
};
2020
use tracing::{error, trace};
@@ -101,8 +101,9 @@ impl SegmentLogWriter {
101101
let batch_size = batch.get_size_bytes();
102102
match confirmation {
103103
Confirmation::Wait => {
104-
let position = self.write_batch(batch).await?;
105-
self.log_size_bytes.store(position, Ordering::Release);
104+
self.write_batch(batch).await?;
105+
self.log_size_bytes
106+
.fetch_add(batch_size.as_bytes_u64(), Ordering::AcqRel);
106107
trace!(
107108
"Written batch of size {batch_size} bytes to log file: {}",
108109
self.file_path
@@ -127,7 +128,7 @@ impl SegmentLogWriter {
127128
}
128129

129130
/// Write a batch of bytes to the log file and return the new file position.
130-
async fn write_batch(&self, batch_to_write: RetainedMessageBatch) -> Result<u64, IggyError> {
131+
async fn write_batch(&self, batch_to_write: RetainedMessageBatch) -> Result<(), IggyError> {
131132
let mut file_guard = self.file.write().await;
132133
if let Some(ref mut file) = *file_guard {
133134
let header = batch_to_write.header_as_bytes();
@@ -141,18 +142,7 @@ impl SegmentLogWriter {
141142
})
142143
.map_err(|_| IggyError::CannotWriteToFile)?;
143144

144-
let new_position = file
145-
.stream_position()
146-
.await
147-
.with_error_context(|e| {
148-
format!(
149-
"Failed to get position of file: {}, error: {e}",
150-
self.file_path
151-
)
152-
})
153-
.map_err(|_| IggyError::CannotReadFileMetadata)?;
154-
155-
Ok(new_position)
145+
Ok(())
156146
} else {
157147
error!("File handle is not available for synchronous write.");
158148
Err(IggyError::CannotWriteToFile)

server/src/streaming/segments/logs/persister_task.rs

+22-30
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
1-
use crate::streaming::batching::message_batch::RetainedMessageBatch;
1+
use crate::streaming::batching::message_batch::{RetainedMessageBatch, RETAINED_BATCH_OVERHEAD};
22
use flume::{unbounded, Receiver};
33
use iggy::{error::IggyError, utils::duration::IggyDuration};
44
use std::{
55
io::IoSlice,
6-
sync::{atomic::AtomicU64, Arc},
6+
sync::{
7+
atomic::{AtomicU64, Ordering},
8+
Arc,
9+
},
710
time::Duration,
811
};
9-
use tokio::{
10-
fs::File,
11-
io::{AsyncSeekExt, AsyncWriteExt},
12-
select,
13-
time::sleep,
14-
};
12+
use tokio::{fs::File, io::AsyncWriteExt, select, time::sleep};
1513
use tracing::{error, trace, warn};
1614

1715
#[derive(Debug)]
@@ -35,12 +33,12 @@ impl PersisterTask {
3533
file: File,
3634
file_path: String,
3735
fsync: bool,
38-
file_size: Arc<AtomicU64>,
36+
log_file_size: Arc<AtomicU64>,
3937
max_retries: u32,
4038
retry_delay: IggyDuration,
4139
) -> Self {
4240
let (sender, receiver) = unbounded();
43-
let file_size_clone = file_size.clone();
41+
let log_file_size_clone = log_file_size.clone();
4442
let file_path_clone = file_path.clone();
4543
let handle = tokio::spawn(async move {
4644
Self::run(
@@ -50,7 +48,7 @@ impl PersisterTask {
5048
fsync,
5149
max_retries,
5250
retry_delay,
53-
file_size_clone,
51+
log_file_size_clone,
5452
)
5553
.await;
5654
});
@@ -171,12 +169,12 @@ impl PersisterTask {
171169
fsync: bool,
172170
max_retries: u32,
173171
retry_delay: IggyDuration,
174-
file_size: Arc<AtomicU64>,
172+
log_file_size: Arc<AtomicU64>,
175173
) {
176174
while let Ok(request) = receiver.recv_async().await {
177175
match request {
178176
PersisterTaskCommand::WriteRequest(batch_to_write) => {
179-
if let Err(e) = Self::write_with_retries(
177+
match Self::write_with_retries(
180178
&mut file,
181179
&file_path,
182180
batch_to_write,
@@ -186,21 +184,14 @@ impl PersisterTask {
186184
)
187185
.await
188186
{
189-
error!(
187+
Ok(bytes_written) => {
188+
log_file_size.fetch_add(bytes_written, Ordering::AcqRel);
189+
}
190+
Err(e) => {
191+
error!(
190192
"Failed to persist data in LogPersisterTask for file {file_path}: {:?}",
191193
e
192-
);
193-
} else {
194-
match file.stream_position().await {
195-
Ok(pos) => {
196-
file_size.store(pos, std::sync::atomic::Ordering::Release);
197-
}
198-
Err(e) => {
199-
error!(
200-
"Failed to get file stream position in LogPersisterTask for file {file_path}: {:?}",
201-
e
202-
);
203-
}
194+
)
204195
}
205196
}
206197
}
@@ -227,18 +218,19 @@ impl PersisterTask {
227218
fsync: bool,
228219
max_retries: u32,
229220
retry_delay: IggyDuration,
230-
) -> Result<(), IggyError> {
221+
) -> Result<u64, IggyError> {
231222
let header = batch_to_write.header_as_bytes();
232223
let batch_bytes = batch_to_write.bytes;
233224
let slices = [IoSlice::new(&header), IoSlice::new(&batch_bytes)];
225+
let bytes_written = RETAINED_BATCH_OVERHEAD + batch_bytes.len() as u64;
234226

235227
let mut attempts = 0;
236228
loop {
237229
match file.write_vectored(&slices).await {
238230
Ok(_) => {
239231
if fsync {
240232
match file.sync_all().await {
241-
Ok(_) => return Ok(()),
233+
Ok(_) => return Ok(bytes_written),
242234
Err(e) => {
243235
attempts += 1;
244236
error!(
@@ -248,7 +240,7 @@ impl PersisterTask {
248240
}
249241
}
250242
} else {
251-
return Ok(());
243+
return Ok(bytes_written);
252244
}
253245
}
254246
Err(e) => {
@@ -265,7 +257,7 @@ impl PersisterTask {
265257
);
266258
return Err(IggyError::CannotWriteToFile);
267259
}
268-
tokio::time::sleep(retry_delay.get_duration()).await;
260+
sleep(retry_delay.get_duration()).await;
269261
}
270262
}
271263
}

0 commit comments

Comments
 (0)