Page MenuHomePhabricator

[Chronik] Shut down node after errors in merge operators gracefully
ClosedPublic

Authored by tobias_ruck on Aug 24 2023, 13:23.

Details

Summary

Currently, when the merge operator full_merge_concat_trim failes, it panics. However, this may cause issues with corrupting the db or even the node.

Therefore, instead of panicing, we log the error message, turn the failed merge into a no-op, store the error in a static global, and handle errors as soon as they arrive (after calling write_batch). See chronik-db/src/io/merge.rs for a rationale and explored alternatives. Note that alternative DB methods (such as using TransactionDB etc.) have been explored, but the behavior is identical to the methods used in Chronik already.

Directly using log! in chronik-db, however, causes linker issues when running cargo test: When chronik-indexer tests are compiled, the dependency chronik-db is *not* compiled with cfg(test) on, because it's a dependency. Therefore, it will compile with the LogPrint function from the node, which in undefined during cargo test.

To avoid this, we set logging functions externally (during setup_chronik) to a OnceLock (static field that can only be set once), which are left to println! during cargo test.

Test Plan

ninja check-crates

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tail of the build log:

 --> chronik/chronik-util/src/log.rs:7:5
  |
7 | use std::sync::OnceLock;
  |     ^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:23:24
   |
23 | static MOUNTED_LOGGER: OnceLock<Loggers> = OnceLock::new();
   |                        ^^^^^^^^^^^^^^^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:23:44
   |
23 | static MOUNTED_LOGGER: OnceLock<Loggers> = OnceLock::new();
   |                                            ^^^^^^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:23:44
   |
23 | static MOUNTED_LOGGER: OnceLock<Loggers> = OnceLock::new();
   |                                            ^^^^^^^^^^^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:33:26
   |
33 |     match MOUNTED_LOGGER.get() {
   |                          ^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:49:26
   |
49 |     match MOUNTED_LOGGER.get() {
   |                          ^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

error[E0658]: use of unstable library feature 'once_cell'
  --> chronik/chronik-util/src/log.rs:60:28
   |
60 |     let _ = MOUNTED_LOGGER.set(loggers);
   |                            ^^^
   |
   = note: see issue #74465 <https://github.com/rust-lang/rust/issues/74465> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `chronik-util` due to 7 previous errors
warning: build failed, waiting for other jobs to finish...
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1
Fabien requested changes to this revision.Aug 24 2023, 15:37

build failed, back to your queue

This revision now requires changes to proceed.Aug 24 2023, 15:37

rebase on master (to get latest Rust 1.72.0)

Fabien requested changes to this revision.Aug 25 2023, 09:53
Fabien added inline comments.
chronik/chronik-db/src/io/merge.rs
1 ↗(On Diff #41941)

Macro whatyearisit:

This revision now requires changes to proceed.Aug 25 2023, 09:53

Turn failed merges into no-ops and handle them after updating the DB, if any

tobias_ruck edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Aug 30 2023, 08:41
tobias_ruck edited the test plan for this revision. (Show Details)

Rebase from master