make nightly clippy happy #2186
Conversation
There was a problem hiding this comment.
Thanks @xudong963 -- looks great. I think we should get an opinion from someone focused on Ballista for the job/stage change, but otherwise good to go from my perspective
| @@ -151,11 +151,11 @@ fn shift_with_default_value( | |||
| create_empty_array(value, array.data_type(), array.len()) | |||
| } else { | |||
| let slice_offset = (-offset).clamp(0, value_len) as usize; | |||
| let length = array.len() - offset.abs() as usize; | |||
| let length = array.len() - offset.unsigned_abs() as usize; | |||
There was a problem hiding this comment.
looking at this code I wonder if we need to check for offset < 0 -- however, this PR seems like a definite improvement so 👍
| @@ -124,28 +124,33 @@ impl<T: 'static + AsLogicalPlan, U: 'static + AsExecutionPlan> | |||
| .get_from_prefix(&get_stage_prefix(&self.namespace)) | |||
| .await?; | |||
|
|
|||
| let mut tmp_stages: HashMap<StageKey, Arc<dyn ExecutionPlan>> = HashMap::new(); | |||
There was a problem hiding this comment.
This makes sense to me (make the snapshot while not holding the lock) but I am not familiar enough with the ballista codebase to know if it would cause problems. Perhaps @mingmwang @yahoNanJing or @thinkharderdev could review it too?
There was a problem hiding this comment.
I don't think so. This should only happen when the scheduler initializes and needs to load the persistent state into memory
| @@ -124,28 +124,33 @@ impl<T: 'static + AsLogicalPlan, U: 'static + AsExecutionPlan> | |||
| .get_from_prefix(&get_stage_prefix(&self.namespace)) | |||
| .await?; | |||
|
|
|||
| let mut tmp_stages: HashMap<StageKey, Arc<dyn ExecutionPlan>> = HashMap::new(); | |||
There was a problem hiding this comment.
I don't think so. This should only happen when the scheduler initializes and needs to load the persistent state into memory
|
Thanks @alamb @thinkharderdev |
abswithunsigned_abs