Skip to content

Commit

Permalink
Update State::advance_time to be less indented
Browse files Browse the repository at this point in the history 
Following @goxberry's PR review on #1057 this PR attempts to remove the
heavy indentation in State::advance_time. Property tests from #1066 still
pass, so I'd say that was solid feedback. The code flows nicer now.

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
  • Loading branch information
@blt
blt committed Oct 29, 2024
1 parent 2fe78db commit f41939f
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 148 deletions.
8 changes: 8 additions & 0 deletions lading/proptest-regressions/generator/file_gen/logrotate_fs/model.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 14b817f8f064c889dabecae41294027ede294a5e51f613ddba46d8bb352f47f5 # shrinks to seed = 18077722532471589563, state = State { nodes: {2: Directory { name: "BUndwQB7", dir: Directory { children: {3}, parent: Some(1) } }, 1: Directory { name: "/", dir: Directory { children: {2}, parent: None } }, 3: File { file: File { parent: 2, bytes_written: 0, bytes_read: 0, access_tick: 0, modified_tick: 0, status_tick: 0, bytes_per_tick: 72, read_only: false, peer: None, ordinal: 0, group_id: 0, open_handles: 0, unlinked: false } }}, root_inode: 1, now: 0, max_rotations: 8, max_bytes_per_file: 32273, group_names: [["77sYYEM2_0.log", "77sYYEM2_0.log.1", "77sYYEM2_0.log.2", "77sYYEM2_0.log.3", "77sYYEM2_0.log.4", "77sYYEM2_0.log.5", "77sYYEM2_0.log.6", "77sYYEM2_0.log.7", "77sYYEM2_0.log.8"]], next_inode: 4, .. }, operations = [GetAttr, Read { offset: 873, size: 578 }, GetAttr, Wait { ticks: 67 }, Close, Open, Close, GetAttr, GetAttr, Open, GetAttr, Wait { ticks: 76 }, Wait { ticks: 90 }, Open, Wait { ticks: 73 }, Wait { ticks: 94 }, Close, Lookup { name: Some("¥'?*:𐠸𝜒A£⳾6?𐢬_Ѩ%ౝ0?{`.𑌂dM") }, Wait { ticks: 63 }, Open, Open, Open, Lookup { name: Some(";0$Ѩ¥𐖘Ⱥ.${*_:gn`Ⱥ\u{113d8}*Z") }, Open, Read { offset: 596, size: 401 }, Read { offset: 314, size: 229 }, Read { offset: 876, size: 934 }, Read { offset: 899, size: 782 }, Read { offset: 871, size: 732 }, Wait { ticks: 3 }, Read { offset: 986, size: 52 }, Lookup { name: None }, Lookup { name: None }, Read { offset: 586, size: 180 }, Close, Open, Lookup { name: Some("𞹾&Ⱥ<") }, GetAttr, Read { offset: 499, size: 626 }, Lookup { name: Some("𑥗%@�^ೋ𝄓") }, Read { offset: 625, size: 519 }, Open, Read { offset: 26, size: 857 }, GetAttr, Read { offset: 530, size: 378 }, Read { offset: 95, size: 717 }, GetAttr, Close, Read { offset: 119, size: 956 }, Open, GetAttr, Read { offset: 760, size: 956 }, Close, Wait { ticks: 98 }, Wait { ticks: 12 }, Read { offset: 138, size: 227 }, Wait { ticks: 41 }, GetAttr]
cc 84a14bb361e5846589558e1fc52c5dee33d22e789034ef13c61f30ca4856d5da # shrinks to seed = 1512443422463708349, state = State { nodes: {1: Directory { name: "/", dir: Directory { children: {2}, parent: None } }, 2: Directory { name: "eKZTyj4p", dir: Directory { children: {3}, parent: Some(1) } }, 3: File { file: File { parent: 2, bytes_written: 0, bytes_read: 0, access_tick: 0, modified_tick: 0, status_tick: 0, bytes_per_tick: 4, read_only: false, peer: None, ordinal: 0, group_id: 0, open_handles: 0, unlinked: false } }}, root_inode: 1, now: 0, max_rotations: 2, max_bytes_per_file: 196227, group_names: [["F5Anm0dg_0.log", "F5Anm0dg_0.log.1", "F5Anm0dg_0.log.2"]], next_inode: 4, .. }, operations = [Wait { ticks: 40 }, Lookup { name: Some("𑌷C&𞺡\"?\"$<&%{$౿ோ") }, GetAttr, GetAttr, GetAttr, GetAttr, GetAttr, GetAttr, Wait { ticks: 17 }, Close, Read { offset: 225, size: 373 }, Wait { ticks: 34 }, Lookup { name: Some("ኻࠕN?¥<q*=%É\u{1d244}₹") }, Lookup { name: None }, Read { offset: 161, size: 111 }, Open, Lookup { name: None }, Wait { ticks: 24 }, Lookup { name: None }, Open, Lookup { name: Some("!*Ѩ𑼍ᛵﮩꬌῼ𞋿יּ:\"") }, Read { offset: 692, size: 769 }, Lookup { name: None }, Lookup { name: Some("𞅅.Ѩf$🉑{lⴧ\\\\?𞸻;સ./⋢ꚱÆᏽ`𐰃𖽻𐖇p𞋿h𝓂m{Z") }, Wait { ticks: 85 }, Open, Read { offset: 220, size: 438 }, Read { offset: 991, size: 393 }, Read { offset: 793, size: 379 }, Lookup { name: None }, GetAttr, Read { offset: 108, size: 606 }, Lookup { name: None }, Lookup { name: Some("𐖠\"n.[") }, Read { offset: 207, size: 867 }, Read { offset: 995, size: 862 }, GetAttr, Read { offset: 633, size: 542 }, Close, Close, Lookup { name: Some("Vպ۞.\"\"𐤈\\磌῞k`'𛲔\"�*|\\'{\u{b82}₻FᱤZP🕴.\\Ⱥ") }, GetAttr, GetAttr, Read { offset: 138, size: 921 }, Wait { ticks: 45 }]
288 changes: 140 additions & 148 deletions lading/src/generator/file_gen/logrotate_fs/model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,175 +508,167 @@ impl State {
///
/// Will panic if passed `now` is less than recorded `now`. Time can only
/// advance.
#[allow(clippy::too_many_lines)]
pub(crate) fn advance_time(&mut self, now: Tick) {
assert!(now >= self.now);
let mut inodes: Vec<Inode> = self.nodes.keys().copied().collect();

for inode in inodes.drain(..) {
// Determine if the file pointed to by inode needs to be rotated. A
// file is only rotated if it is linked, that is, it has a name in
// the filesystem.
let rotation_data = {
if let Some(node) = self.nodes.get_mut(&inode) {
match node {
Node::File { file } => {
file.advance_time(now);
if file.read_only() {
None
} else if file.size() >= self.max_bytes_per_file {
// Because of the way read-only is set on any
// but the 0th ordinal the only read/write file
// that ever is detected for rotation is the 0th
// ordinal.
assert!(
file.ordinal() == 0,
"Expected rotated file to be 0th ordinal, was {ordinal}",
ordinal = file.ordinal()
);
file.set_read_only(now);
Some((
inode,
file.parent,
file.bytes_per_tick,
file.group_id,
file.ordinal(),
))
} else {
None
}
}
Node::Directory { .. } => None,
}
} else {
// Node has been removed, skip
let (rotated_inode, parent_inode, bytes_per_tick, group_id, ordinal) = {
// If the node pointed to by inode doesn't exist, that's a
// catastrophic programming error. We just copied all inode to node
// pairs.
let node = self
.nodes
.get_mut(&inode)
.expect("inode not associated with node");

// Only process files.
let file = match node {
Node::File { file } => file,
Node::Directory { .. } => continue,
};

// No matter what we advance time for the file.
file.advance_time(now);

// If the file is read-only we have no more work to do on this file
// although it _may_ be touched if we process a peer chain below.
if file.read_only() {
continue;
}
};

// The file pointed to by `inode` -- which we will refer to as
// `rotated_inode` from this point on -- is to be rotated. A
// rotated file is not immediately unlinked but it does have its
// ordinal increased which may cause it to be unlinked if the
// ordinal exceeds `max_rotations`. An unlinked file may be deleted
// if there are no active file handles. A rotated file is set
// read-only so it never accumulates further bytes.
if let Some((rotated_inode, parent_inode, bytes_per_tick, group_id, ordinal)) =
rotation_data
{
// Create our new file, called, well, `new_file`. This will
// become the 0th ordinal in the `group_id` and may -- although
// we don't know yet -- cause `rotated_inode` to be deleted.
let new_file_inode = self.next_inode;
let mut new_file = File {
parent: parent_inode,
bytes_written: 0,
bytes_read: 0,
access_tick: self.now,
modified_tick: self.now,
status_tick: self.now,
created_tick: self.now,
bytes_per_tick,
read_only: false,
ordinal: 0,
peer: Some(rotated_inode),
group_id,
open_handles: 0,
unlinked: false,
max_offset_observed: 0,
read_only_since: None,
};
new_file.advance_time(now);

// Insert `new_file` into the node list and make it a member of
// its directory's children.
self.nodes
.insert(new_file_inode, Node::File { file: new_file });
if let Some(Node::Directory { dir, .. }) = self.nodes.get_mut(&parent_inode) {
dir.children.insert(new_file_inode);
// Determine if the file pointed to by inode needs to be rotated. A
// file is only rotated if it is linked, that is, it has a name in
// the filesystem.
if file.size() < self.max_bytes_per_file {
continue;
}
assert!(
file.ordinal() == 0,
"Expected rotated file to be 0th ordinal, was {}",
file.ordinal()
);
file.set_read_only(now);

// Rotation data needed below.
(
inode,
file.parent,
file.bytes_per_tick,
file.group_id,
file.ordinal,
)
};

// Bump the Inode index
self.next_inode = self.next_inode.saturating_add(1);

// Now, we step through the list of peers beginning with
// `rotated_inode` and increment the ordinal of each file we
// find. This tells us if the file falls off the end of the peer
// chain or not. If it does, the file will be unlinked. If the
// file is unlinked and has no active file handles it is removed
// from the node map.
//
// We do not preserve unlinked files in the peer chain. We
// likewise do not preserve unlinked files in its parent
// directory. We _do_ preserve unlinked files in the node map
// until the last file handle is removed for that node.

let mut current_inode = rotated_inode;
assert!(ordinal == 0, "Expected ordinal 0, got {ordinal}");
let mut prev_inode = new_file_inode;

loop {
// Increment the current_inode's ordinal and determine if
// the ordinal is now past max_rotations and whether the
// next peer needs to be followed.
let (remove_current, next_peer) = {
let node = self.nodes.get_mut(&current_inode).expect("Node must exist");
match node {
Node::File { file } => {
file.incr_ordinal();

let remove_current = file.ordinal() > self.max_rotations;
(remove_current, file.peer)
}
Node::Directory { .. } => panic!("Expected a File node"),
}
};
// Create our new file, called, well, `new_file`. This will
// become the 0th ordinal in the `group_id` and may -- although
// we don't know yet -- cause `rotated_inode` to be deleted.
let new_file_inode = self.next_inode;
let mut new_file = File {
parent: parent_inode,
bytes_written: 0,
bytes_read: 0,
access_tick: self.now,
modified_tick: self.now,
status_tick: self.now,
created_tick: self.now,
bytes_per_tick,
read_only: false,
ordinal: 0,
peer: Some(rotated_inode),
group_id,
open_handles: 0,
unlinked: false,
max_offset_observed: 0,
read_only_since: None,
};
new_file.advance_time(now);
self.next_inode = self.next_inode.saturating_add(1);

// Insert `new_file` into the node list and make it a member of
// its directory's children.
self.nodes
.insert(new_file_inode, Node::File { file: new_file });
if let Some(Node::Directory { dir, .. }) = self.nodes.get_mut(&parent_inode) {
dir.children.insert(new_file_inode);
}

if remove_current {
// The only time a node is removed is when it's at the
// end of the peer chain. This means that next_peer is
// None and there are no further peers to explore.
assert!(next_peer.is_none());

// Because we are at the end of the peer chain we remove
// the node from its parent directory and unlink it.
if let Some(Node::Directory { dir, .. }) = self.nodes.get_mut(&parent_inode)
{
dir.children.remove(&current_inode);
}
if let Some(Node::File { file }) = self.nodes.get_mut(&current_inode) {
file.unlink(now);
}
// Now, we step through the list of peers beginning with
// `rotated_inode` and increment the ordinal of each file we
// find. This tells us if the file falls off the end of the peer
// chain or not. If it does, the file will be unlinked. If the
// file is unlinked and has no active file handles it is removed
// from the node map.
//
// We do not preserve unlinked files in the peer chain. We
// likewise do not preserve unlinked files in its parent
// directory. We _do_ preserve unlinked files in the node map
// until the last file handle is removed for that node.

let mut current_inode = rotated_inode;
assert!(ordinal == 0, "Expected ordinal 0, got {ordinal}");
let mut prev_inode = new_file_inode;

loop {
// Increment the current_inode's ordinal and determine if
// the ordinal is now past max_rotations and whether the
// next peer needs to be followed.
let node = self.nodes.get_mut(&current_inode).expect("Node must exist");
let (remove_current, next_peer) = match node {
Node::File { file } => {
file.incr_ordinal();

let remove_current = file.ordinal() > self.max_rotations;
(remove_current, file.peer)
}
Node::Directory { .. } => panic!("Expected a File node"),
};

// Update the peer of the previous file to None
let node = self.nodes.get_mut(&prev_inode).expect("Node must exist");
if let Node::File { file } = node {
file.peer = None;
}
if remove_current {
// The only time a node is removed is when it's at the end
// of the peer chain. This means that next_peer is None and
// there are no further peers to explore.
assert!(
next_peer.is_none(),
"next_peer must be None when removing a node, else not end of peer chain"
);

// Break the loop, as there are no further peers to process
break;
// Because we are at the end of the peer chain we remove
// the node from its parent directory and unlink it.
if let Some(Node::Directory { dir, .. }) = self.nodes.get_mut(&parent_inode) {
dir.children.remove(&current_inode);
}
if let Some(Node::File { file }) = self.nodes.get_mut(&current_inode) {
file.unlink(now);
}

// Move to the next peer
//
// The next_peer is only None in the `remove_current`
// branch, meaning that we only reach this point if the next
// peer is Some.
prev_inode = current_inode;
if let Some(next_peer) = next_peer {
current_inode = next_peer;
} else {
// We're at the end of the rotated files but not so many
// it's time to rotate off.
break;
// Update the peer of the previous file to None
let node = self.nodes.get_mut(&prev_inode).expect("Node must exist");
if let Node::File { file } = node {
file.peer = None;
}

// Break the loop, as there are no further peers to process
break;
}

// Move to the next peer
//
// The next_peer is only None in the `remove_current`
// branch, meaning that we only reach this point if the next
// peer is Some.
prev_inode = current_inode;
if let Some(next_peer) = next_peer {
current_inode = next_peer;
} else {
// We're at the end of the rotated files but not so many
// it's time to rotate off.
break;
}
}
}

self.gc();

self.now = now;
}

Expand Down

0 comments on commit f41939f

Please sign in to comment.