Skip to content

fix: don't bail on room compression #155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: don't bail on room compression #155

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion synapse_auto_compressor/src/manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
connect_to_database, create_tables_if_needed, get_next_room_to_compress,
read_room_compressor_state, write_room_compressor_state,
};
use anyhow::{bail, Context, Result};

Check warning on line 8 in synapse_auto_compressor/src/manager.rs

View workflow job for this annotation

GitHub Actions / Test Suite 

unused import: `bail`
use log::{debug, info, warn};
use synapse_compress_state::{continue_run, ChunkStats, Level};

Expand Down Expand Up @@ -183,7 +183,9 @@
}
chunks_processed += 1;
} else {
bail!("Ran the compressor on a room that had no more work to do!")
// Room is already fully compressed, skip to next room
debug!("Room {} is already fully compressed, moving to next room", room_to_compress);
continue;
Comment on lines +186 to +188
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you tested this in real life?

It looks like this risks hot-looping on the same room again and again — after all, we just selected a new room to compress (therefore the query for finding compressible rooms believed it was compressible), we didn't modify the room in any way and now we're going to select a room to compress... what stops us from getting the same room again?

Bear in mind I'm not familiar with this codebase to be fair — but this seems to be my impression of the intention of the code — this bail is indicating a bug that we are selecting a room believing it is compressible but actually it isnt..

}
}
info!(
Expand Down
Loading