Skip to content

Commit

Permalink
Use delete_range instead of clear() to avoid race condition (#12855)
Browse files Browse the repository at this point in the history 
because clear() drops and re-creates the table without holding a lock,
it can cause crashes if other threads happen to try to write to or read
from the table being cleared. I don't think it makes sense to add
locking to every DBMap access just to fix this case, so I'm just going
to avoid calling clear().
  • Loading branch information
@mystenmark
mystenmark authored Jul 6, 2023
1 parent 3c13195 commit 199e0b0
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions crates/sui-core/src/checkpoints/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,17 @@ impl CheckpointStore {
}

fn prune_local_summaries(&self) -> SuiResult {
self.locally_computed_checkpoints.clear()?;
info!("Pruned all local checkpoint summaries at end of epoch");
if let Some((last_local_summary, _)) = self
.locally_computed_checkpoints
.unbounded_iter()
.skip_to_last()
.next()
{
let mut batch = self.locally_computed_checkpoints.batch();
batch.delete_range(&self.locally_computed_checkpoints, &0, &last_local_summary)?;
batch.write()?;
info!("Pruned local summaries up to {:?}", last_local_summary);
}
Ok(())
}

Expand Down

1 comment on commit 199e0b0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

4 Validators 500/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 595 | 595 | 0      | 38            | 8035          | 8711          | 473,410,176,000       | 28,404,610,560,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 13  | 100 |

4 Validators 500/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 527 | 527 | 0      | 14            | 7951          | 15295         | 493,275,910,800       | 29,596,554,648,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 14  | 100 |

20 Validators 50/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 200 | 200 | 0      | 22            | 65            | 128           | 160,656,384,000       | 9,639,383,040,000          |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 27  | 51  |

20 Validators 50/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 193 | 193 | 0      | 56            | 1423          | 2061          | 185,748,754,800       | 11,144,925,288,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 25  | 55  |

Narwhal Benchmark Results

 SUMMARY:
-----------------------------------------
 + CONFIG:
 Faults: 0 node(s)
 Committee size: 4 node(s)
 Worker(s) per node: 1 worker(s)
 Collocate primary and workers: True
 Input rate: 50,000 tx/s
 Transaction size: 512 B
 Execution time: 0 s

 Header number of batches threshold: 32 digests
 Header maximum number of batches: 1,000 digests
 Max header delay: 2,000 ms
 GC depth: 50 round(s)
 Sync retry delay: 10,000 ms
 Sync retry nodes: 3 node(s)
 batch size: 500,000 B
 Max batch delay: 200 ms
 Max concurrent requests: 500,000 

 + RESULTS:
 Batch creation avg latency: 201 ms
 Header creation avg latency: -1 ms
 	Batch to header avg latency: -1 ms
 Header to certificate avg latency: 1 ms
 	Request vote outbound avg latency: 0 ms
 Certificate commit avg latency: 713 ms

 Consensus TPS: 0 tx/s
 Consensus BPS: 0 B/s
 Consensus latency: 0 ms

 End-to-end TPS: 0 tx/s
 End-to-end BPS: 0 B/s
 End-to-end latency: 0 ms
-----------------------------------------

Please sign in to comment.