influxdata / influxdata/influxdb
Application of catalog batches should be infallible
- Dominant language
- Rust
- Stars
- 31.7k
- Forks
- 3.7k
- Avg merge
- 13h 37m
- Merged PRs (30d)
- 8
Description
### Problem
There is a method in the catalog whose API is fallible, i.e., returns `Result`: https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L978-L983
This API should not be fallible, because, it is invoked after the provided `CatalogBatch` has been persisted to object store. Therefore, the `CatalogBatch` should already have been validated, and applying it to the in-memory catalog _must_ succeed.
If application does not succeed, then the code should `panic!`, because things are in an unexpected state.
Furthermore, having that and other `apply_*` methods return `Result` indicates to future developers that those methods are allowed to fail, which may lead to bugs.
### Proposed solution
Change the following APIs to not return `Result`, and panic on fallible operations:
* [`apply_catalog_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L979) - a comment/note should be added to this method explaining why it is not fallible
* [`apply_node_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1002)
* [`apply_token_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1065)
* [`apply_database_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1107)
### Additional context
See conversation starting with https://github.com/influxdata/influxdb_pro/pull/910#discussion_r2132850379.
Contributor guide
Research direction
Start in influxdb3_catalog/src/catalog.rs at apply_catalog_batch and the three related apply_*_batch methods. Trace their callers and the existing fallible operations, then verify the catalog crate still builds and its tests pass; done means these APIs no longer return Result, unexpected failures panic, and apply_catalog_batch documents why.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100