Skip to content

Commit

Permalink
update docs and tolerate placeholder for incomplete catalogs
Browse files Browse the repository at this point in the history
Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao committed Oct 21, 2024
1 parent c644a1b commit ad6d5f5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 16 deletions.
12 changes: 6 additions & 6 deletions proto/catalog.proto
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,13 @@ message Table {

// Total vnode count of the table.
//
// Can be unset if...
// - The catalog is generated by the frontend and not yet persisted, this is
// because the vnode count of each fragment (then internal tables) is determined
// by the meta service.
// - The table is created in older versions where variable vnode count is not
// Use `VnodeCountCompat::vnode_count` to access it.
//
// - Can be unset if the table is created in older versions where variable vnode count is not
// supported, in which case a default value of 256 should be used.
// Use `VnodeCountCompat::vnode_count` to access it.
// - Can be placeholder value `Some(0)` if the catalog is generated by the frontend and the
// corresponding job is still in `Creating` status, in which case calling `vnode_count`
// will panic.
//
// Please note that this field is not intended to describe the expected vnode count
// for a streaming job. Instead, refer to `stream_plan.StreamFragmentGraph.max_parallelism`.
Expand Down
22 changes: 12 additions & 10 deletions src/frontend/src/catalog/table_catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::assert_matches::assert_matches;
use std::collections::{HashMap, HashSet};

use fixedbitset::FixedBitSet;
Expand Down Expand Up @@ -174,14 +175,9 @@ pub struct TableCatalog {

/// Total vnode count of the table.
///
/// Can be placeholder if the catalog is generated by the frontend and not yet persisted. This is
/// because the vnode count of each fragment (then internal tables) is determined by the
/// meta service. In this case, calling [`TableCatalog::vnode_count()`] will panic.
/// See also [`StreamMaterialize::derive_table_catalog`] and [`TableCatalogBuilder::build`].
///
/// On the contrary, if this comes from a [`PbTable`], the field must have value no matter
/// whether the table is created before or after the version we introduced variable vnode
/// count support. We will handle backward compatibility when obtaining the value.
/// Can be [`VnodeCount::Placeholder`] if the catalog is generated by the frontend and the
/// corresponding job is still in `Creating` status, in which case calling [`Self::vnode_count`]
/// will panic.
///
/// [`StreamMaterialize::derive_table_catalog`]: crate::optimizer::plan_node::StreamMaterialize::derive_table_catalog
/// [`TableCatalogBuilder::build`]: crate::optimizer::plan_node::utils::TableCatalogBuilder::build
Expand Down Expand Up @@ -561,7 +557,13 @@ impl From<PbTable> for TableCatalog {
OptionalAssociatedSourceId::AssociatedSourceId(id) => id,
});
let name = tb.name.clone();
let vnode_count = tb.vnode_count();

let vnode_count = tb.vnode_count_inner();
if let VnodeCount::Placeholder = vnode_count {
// Only allow placeholder vnode count for creating tables.
// After the table is created, an `Update` notification will be used to update the vnode count field.
assert_matches!(stream_job_status, PbStreamJobStatus::Creating);
}

let mut col_names = HashSet::new();
let mut col_index: HashMap<i32, usize> = HashMap::new();
Expand Down Expand Up @@ -632,7 +634,7 @@ impl From<PbTable> for TableCatalog {
.map(TableId::from)
.collect_vec(),
cdc_table_id: tb.cdc_table_id,
vnode_count: VnodeCount::set(vnode_count), /* from existing (persisted) tables, vnode_count must be set */
vnode_count,
}
}
}
Expand Down

0 comments on commit ad6d5f5

Please sign in to comment.