Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adapter: Convert system config to unsigned ints #14502

Merged
merged 2 commits into from
Aug 29, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fix casting and overflow
  • Loading branch information
jkosh44 committed Aug 29, 2022
commit 6fcc98bb63fde960f1cfb6abe541b6dce572eebd
23 changes: 16 additions & 7 deletions src/adapter/src/coord/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use tracing::Level;
use tracing::{event, warn};

use mz_compute_client::controller::{ComputeInstanceId, ComputeSinkId};
use mz_ore::cast::CastFrom;
use mz_ore::retry::Retry;
use mz_ore::task;
use mz_repr::{GlobalId, Timestamp};
Expand Down Expand Up @@ -666,19 +665,29 @@ impl<S: Append + 'static> Coordinator<S> {
fn validate_resource_limit<F>(
&self,
current_amount: usize,
new_instances: u32,
new_instances: i32,
resource_limit: F,
resource_type: &str,
) -> Result<(), AdapterError>
where
F: Fn(&SystemVars) -> u32,
{
let limit = resource_limit(self.catalog.state().system_config());
if current_amount > usize::cast_from(u32::MAX)
|| (new_instances > 0
&& u32::try_from(current_amount).expect("guaranteed to fit") + new_instances
> limit)
{
let exceeds_limit = match (u32::try_from(current_amount), u32::try_from(new_instances)) {
// 0 new instances are always ok.
(_, Ok(new_instances)) if new_instances == 0 => false,
// negative instances are always ok.
(_, Err(_)) => false,
// more than u32 for the current amount is too much.
(Err(_), _) => true,
(Ok(current_amount), Ok(new_instances)) => {
match current_amount.checked_add(new_instances) {
Some(new_amount) => new_amount > limit,
None => true,
}
}
};
if exceeds_limit {
Err(AdapterError::ResourceExhaustion {
resource_type: resource_type.to_string(),
limit,
Expand Down