About the execution order of compact and create_snapshot

[yufan022]

Hi @PsiACE , thanks for your contribution.

i'm reading the code of this repository, and i have a question about here:

riteraft/src/raft_node.rs

Lines 479 to 488 in 56d53b4

	if let Ok(cs) = self.apply_conf_change(&change) {
	let last_applied = self.raft.raft_log.applied;
	let snapshot = self.store.snapshot().await?;
	{
	let store = self.mut_store();
	store.set_conf_state(&cs)?;
	store.compact(last_applied)?;
	store.create_snapshot(snapshot)?;
	}
	}

In my understanding, it should be ensured that the snapshot is successfully created before executing compact to clean up older raft-logs to ensure safety.

However, I see that the code executes the compact before executing the create_snapshot. Would this cause some issues if compact is executed first but the snapshot creation fails? Is my concern reasonable?

store.compact(last_applied)?; // already clean the older logs
store.create_snapshot(snapshot)?; // but create_snapshot failed

Is it a better choice to create_snapshot before executing compact?

store.create_snapshot(snapshot)?;
store.compact(last_applied)?; // only create_snapshot success, then clean the older logs

[PsiACE]

Your concern is valid. Creating the snapshot before performing compaction is indeed a safer order. Alternatively, compaction could be implemented as a "mark for deletion" operation, with the actual log deletion only happening after the snapshot is successfully created.

It's been a while since I last revisited this part of the code—thank you for your attention and thoughtful question!

[yufan022]

Thank you for your explanation and advice, which is very enlightening to me!