Skip to content

Commit 7eb5934

Browse files
authored
style: prefer stack pinning (#9786)
Signed-off-by: TennyZhuang <[email protected]>
1 parent 49db6a8 commit 7eb5934

File tree

8 files changed

+139
-125
lines changed

8 files changed

+139
-125
lines changed

grafana/generate.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ for dashboard_name in "risingwave-dev-dashboard" "risingwave-user-dashboard"; do
66
generate-dashboard -o $dashboard_name.gen.json $dashboard_name.dashboard.py
77
jq -c . $dashboard_name.gen.json > $dashboard_name.json
88
cp $dashboard_name.json ../docker/dashboards/
9-
done
9+
done

src/common/src/util/stream_cancel.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,16 @@
1919
//!
2020
//! Following is an example of how it works:
2121
//! ```rust
22+
//! use std::pin::pin;
23+
//!
2224
//! use futures::stream::iter;
2325
//! use futures::StreamExt;
2426
//! use risingwave_common::util::stream_cancel::{cancellable_stream, stream_tripwire};
2527
//!
2628
//! #[tokio::main]
2729
//! async fn main() {
2830
//! let (trigger, tripwire) = stream_tripwire(|| 5);
29-
//! let mut s = Box::pin(cancellable_stream(iter(vec![1i32, 2, 3, 4]), tripwire));
31+
//! let mut s = pin!(cancellable_stream(iter(vec![1i32, 2, 3, 4]), tripwire));
3032
//! assert_eq!(Some(1), s.next().await);
3133
//! assert_eq!(Some(2), s.next().await);
3234
//! trigger.abort();
@@ -113,6 +115,8 @@ where
113115

114116
#[cfg(test)]
115117
mod tests {
118+
use std::pin::pin;
119+
116120
use futures::stream::iter;
117121
use futures::StreamExt;
118122

@@ -121,7 +125,7 @@ mod tests {
121125
#[tokio::test]
122126
async fn test_cancellable_stream_aborted() {
123127
let (trigger, tripwire) = stream_tripwire(|| 5);
124-
let mut s = Box::pin(cancellable_stream(iter(vec![1i32, 2, 3, 4]), tripwire));
128+
let mut s = pin!(cancellable_stream(iter(vec![1i32, 2, 3, 4]), tripwire));
125129
assert_eq!(Some(1), s.next().await);
126130
assert_eq!(Some(2), s.next().await);
127131
trigger.abort();
@@ -133,7 +137,7 @@ mod tests {
133137
#[tokio::test]
134138
async fn test_cancellable_stream_not_aborted() {
135139
let (trigger, tripwire) = stream_tripwire(|| 5);
136-
let mut s = Box::pin(cancellable_stream(iter(vec![1, 2, 3, 4]), tripwire));
140+
let mut s = pin!(cancellable_stream(iter(vec![1, 2, 3, 4]), tripwire));
137141
assert_eq!(Some(1), s.next().await);
138142
assert_eq!(Some(2), s.next().await);
139143
assert_eq!(Some(3), s.next().await);

src/connector/src/source/filesystem/s3/source/reader.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
use std::collections::HashMap;
16+
use std::pin::pin;
1617

1718
use anyhow::{anyhow, Result};
1819
use async_trait::async_trait;
@@ -69,7 +70,7 @@ impl S3FileReader {
6970
byte_stream.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e)),
7071
);
7172

72-
let reader = Box::pin(BufReader::new(stream_reader));
73+
let reader = pin!(BufReader::new(stream_reader));
7374

7475
let stream = ReaderStream::with_capacity(reader, STREAM_READER_CAPACITY);
7576

@@ -202,9 +203,9 @@ impl S3FileReader {
202203
parser,
203204
ByteStreamSourceParserImpl::Json(_) | ByteStreamSourceParserImpl::Csv(_)
204205
) {
205-
NdByteStreamWrapper::new(parser).into_stream(Box::pin(data_stream))
206+
NdByteStreamWrapper::new(parser).into_stream(data_stream)
206207
} else {
207-
parser.into_stream(Box::pin(data_stream))
208+
parser.into_stream(data_stream)
208209
};
209210
#[for_await]
210211
for msg in msg_stream {

src/stream/src/executor/backfill.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use std::cmp::Ordering;
1616
use std::ops::Bound;
17+
use std::pin::pin;
1718
use std::sync::Arc;
1819

1920
use await_tree::InstrumentAwait;
@@ -192,10 +193,13 @@ where
192193

193194
let left_upstream = upstream.by_ref().map(Either::Left);
194195

195-
let right_snapshot = Box::pin(
196-
Self::snapshot_read(&self.table, snapshot_read_epoch, current_pos.clone(), true)
197-
.map(Either::Right),
198-
);
196+
let right_snapshot = pin!(Self::snapshot_read(
197+
&self.table,
198+
snapshot_read_epoch,
199+
current_pos.clone(),
200+
true
201+
)
202+
.map(Either::Right),);
199203

200204
// Prefer to select upstream, so we can stop snapshot stream as soon as the barrier
201205
// comes.

src/stream/src/executor/lookup/sides.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
use std::pin::pin;
16+
1517
use anyhow::Context;
1618
use either::Either;
1719
use futures::stream::PollNext;
@@ -122,8 +124,8 @@ fn prefer_right(_: &mut ()) -> PollNext {
122124
/// available for both left and right side, instead of being combined.
123125
#[try_stream(ok = BarrierAlignedMessage, error = StreamExecutorError)]
124126
pub async fn align_barrier(left: impl MessageStream, right: impl MessageStream) {
125-
let mut left = Box::pin(left);
126-
let mut right = Box::pin(right);
127+
let mut left = pin!(left);
128+
let mut right = pin!(right);
127129

128130
enum SideStatus {
129131
LeftBarrier,
@@ -201,7 +203,7 @@ pub async fn stream_lookup_arrange_prev_epoch(
201203
stream: Box<dyn Executor>,
202204
arrangement: Box<dyn Executor>,
203205
) {
204-
let mut input = Box::pin(align_barrier(stream.execute(), arrangement.execute()));
206+
let mut input = pin!(align_barrier(stream.execute(), arrangement.execute()));
205207
let mut arrange_buf = vec![];
206208
let mut stream_side_end = false;
207209

@@ -285,7 +287,7 @@ pub async fn stream_lookup_arrange_this_epoch(
285287
stream: Box<dyn Executor>,
286288
arrangement: Box<dyn Executor>,
287289
) {
288-
let mut input = Box::pin(align_barrier(stream.execute(), arrangement.execute()));
290+
let mut input = pin!(align_barrier(stream.execute(), arrangement.execute()));
289291
let mut stream_buf = vec![];
290292
let mut arrange_buf = vec![];
291293

src/stream/src/executor/rearranged_chain.rs

Lines changed: 90 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
use std::pin::pin;
16+
1517
use futures::channel::{mpsc, oneshot};
1618
use futures::stream::select_with_strategy;
1719
use futures::{stream, StreamExt};
@@ -105,7 +107,7 @@ impl RearrangedChainExecutor {
105107

106108
#[try_stream(ok = Message, error = StreamExecutorError)]
107109
async fn execute_inner(mut self) {
108-
let mut upstream = Box::pin(self.upstream.execute());
110+
let mut upstream = pin!(self.upstream.execute());
109111

110112
// 1. Poll the upstream to get the first barrier.
111113
let first_barrier = expect_first_barrier(&mut upstream).await?;
@@ -133,106 +135,107 @@ impl RearrangedChainExecutor {
133135
.unbounded_send(RearrangedMessage::PhantomBarrier(first_barrier))
134136
.unwrap();
135137

136-
// 3. Rearrange stream, will yield the barriers polled from upstream to rearrange.
137-
let rearranged_barrier = Box::pin(
138-
Self::rearrange_barrier(&mut upstream, upstream_tx, stop_rearrange_rx)
139-
.map(|result| result.map(RearrangedMessage::RearrangedBarrier)),
140-
);
141-
142-
// 4. Init the snapshot with reading epoch.
143-
let snapshot = self.snapshot.execute_with_epoch(create_epoch.prev);
144-
145-
// Chain the `snapshot` and `upstream_rx` to get a unified `rearranged_chunks` stream.
146-
let rearranged_chunks = snapshot
147-
.map(|result| result.map(RearrangedMessage::rearranged_from))
148-
.chain(upstream_rx.map(Ok));
149-
150-
// 5. Merge the rearranged barriers with chunks, with the priority of barrier.
151-
let mut rearranged =
152-
select_with_strategy(rearranged_barrier, rearranged_chunks, |_: &mut ()| {
153-
stream::PollNext::Left
154-
});
155-
156-
// Record the epoch of the last rearranged barrier we received.
157-
let mut last_rearranged_epoch = create_epoch;
158-
let mut stop_rearrange_tx = Some(stop_rearrange_tx);
159-
160-
let mut processed_rows: u64 = 0;
161-
162-
// 6. Consume the merged `rearranged` stream.
163-
#[for_await]
164-
for rearranged_msg in &mut rearranged {
165-
match rearranged_msg? {
166-
// If we received a phantom barrier, update the progress and check whether we
167-
// catches up with the progress of upstream MV.
168-
//
169-
// Note that there's no phantom barrier in the snapshot. So we must have already
170-
// consumed the whole snapshot and be on the upstream now.
171-
RearrangedMessage::PhantomBarrier(barrier) => {
172-
// Update the progress since we've consumed all chunks before this phantom.
173-
self.progress.update(
174-
last_rearranged_epoch.curr,
175-
barrier.epoch.curr,
176-
processed_rows,
177-
);
178-
179-
if barrier.epoch.curr >= last_rearranged_epoch.curr {
180-
// Stop the background rearrangement task.
181-
stop_rearrange_tx.take().unwrap().send(()).map_err(|_| {
182-
StreamExecutorError::channel_closed("stop rearrange")
183-
})?;
184-
break;
138+
{
139+
// 3. Rearrange stream, will yield the barriers polled from upstream to rearrange.
140+
let rearranged_barrier =
141+
pin!(
142+
Self::rearrange_barrier(&mut upstream, upstream_tx, stop_rearrange_rx)
143+
.map(|result| result.map(RearrangedMessage::RearrangedBarrier)),
144+
);
145+
146+
// 4. Init the snapshot with reading epoch.
147+
let snapshot = self.snapshot.execute_with_epoch(create_epoch.prev);
148+
149+
// Chain the `snapshot` and `upstream_rx` to get a unified `rearranged_chunks`
150+
// stream.
151+
let rearranged_chunks = snapshot
152+
.map(|result| result.map(RearrangedMessage::rearranged_from))
153+
.chain(upstream_rx.map(Ok));
154+
155+
// 5. Merge the rearranged barriers with chunks, with the priority of barrier.
156+
let mut rearranged =
157+
select_with_strategy(rearranged_barrier, rearranged_chunks, |_: &mut ()| {
158+
stream::PollNext::Left
159+
});
160+
161+
// Record the epoch of the last rearranged barrier we received.
162+
let mut last_rearranged_epoch = create_epoch;
163+
let mut stop_rearrange_tx = Some(stop_rearrange_tx);
164+
165+
let mut processed_rows: u64 = 0;
166+
167+
#[for_await]
168+
for rearranged_msg in &mut rearranged {
169+
match rearranged_msg? {
170+
// If we received a phantom barrier, update the progress and check whether
171+
// we catches up with the progress of upstream MV.
172+
//
173+
// Note that there's no phantom barrier in the snapshot. So we must have
174+
// already consumed the whole snapshot and be on the
175+
// upstream now.
176+
RearrangedMessage::PhantomBarrier(barrier) => {
177+
// Update the progress since we've consumed all chunks before this
178+
// phantom.
179+
self.progress.update(
180+
last_rearranged_epoch.curr,
181+
barrier.epoch.curr,
182+
processed_rows,
183+
);
184+
185+
if barrier.epoch.curr >= last_rearranged_epoch.curr {
186+
// Stop the background rearrangement task.
187+
stop_rearrange_tx.take().unwrap().send(()).map_err(|_| {
188+
StreamExecutorError::channel_closed("stop rearrange")
189+
})?;
190+
break;
191+
}
185192
}
186-
}
187193

188-
// If we received a message, yield it.
189-
RearrangedMessage::RearrangedBarrier(barrier) => {
190-
last_rearranged_epoch = barrier.epoch;
191-
yield Message::Barrier(barrier);
192-
}
193-
RearrangedMessage::Chunk(chunk) => {
194-
processed_rows += chunk.cardinality() as u64;
195-
yield Message::Chunk(chunk)
196-
}
197-
RearrangedMessage::Watermark => {
198-
// Ignore watermark during snapshot consumption.
194+
// If we received a message, yield it.
195+
RearrangedMessage::RearrangedBarrier(barrier) => {
196+
last_rearranged_epoch = barrier.epoch;
197+
yield Message::Barrier(barrier);
198+
}
199+
RearrangedMessage::Chunk(chunk) => {
200+
processed_rows += chunk.cardinality() as u64;
201+
yield Message::Chunk(chunk)
202+
}
203+
RearrangedMessage::Watermark => {
204+
// Ignore watermark during snapshot consumption.
205+
}
199206
}
200207
}
201-
}
202-
203-
// 7. Rearranged task finished.
204-
// The reason for finish must be that we told it to stop.
205-
tracing::trace!(actor = self.actor_id, "rearranged task finished");
206-
if stop_rearrange_tx.is_some() {
207-
tracing::error!(actor = self.actor_id, "rearrangement finished passively");
208-
}
209208

210-
// 8. Consume remainings.
211-
let mut finish_on_barrier = |msg: &Message| {
212-
if let Some(barrier) = msg.as_barrier() {
213-
self.progress.finish(barrier.epoch.curr);
209+
// 7. Rearranged task finished.
210+
// The reason for finish must be that we told it to stop.
211+
tracing::trace!(actor = self.actor_id, "rearranged task finished");
212+
if stop_rearrange_tx.is_some() {
213+
tracing::error!(actor = self.actor_id, "rearrangement finished passively");
214214
}
215-
};
216215

217-
// Note that there may still be some messages in `rearranged`. However the rearranged
218-
// barriers must be ignored, we should take the phantoms.
219-
#[for_await]
220-
for msg in rearranged {
221-
let msg: RearrangedMessage = msg?;
222-
let Some(msg) = msg.phantom_into() else { continue };
223-
finish_on_barrier(&msg);
224-
yield msg;
216+
// 8. Consume remainings.
217+
// Note that there may still be some messages in `rearranged`. However the
218+
// rearranged barriers must be ignored, we should take the phantoms.
219+
#[for_await]
220+
for msg in rearranged {
221+
let msg: RearrangedMessage = msg?;
222+
let Some(msg) = msg.phantom_into() else { continue };
223+
if let Some(barrier) = msg.as_barrier() {
224+
self.progress.finish(barrier.epoch.curr);
225+
}
226+
yield msg;
227+
}
225228
}
226229

227-
let mut remaining_upstream = upstream;
228-
229230
// Consume remaining upstream.
230231
tracing::trace!(actor = self.actor_id, "begin to consume remaining upstream");
231232

232233
#[for_await]
233-
for msg in &mut remaining_upstream {
234+
for msg in upstream {
234235
let msg: Message = msg?;
235-
finish_on_barrier(&msg);
236+
if let Some(barrier) = msg.as_barrier() {
237+
self.progress.finish(barrier.epoch.curr);
238+
}
236239
yield msg;
237240
}
238241
} else {

src/stream/src/executor/temporal_join.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
use std::alloc::Global;
16+
use std::pin::pin;
1617
use std::sync::Arc;
1718

1819
use either::Either;
@@ -124,8 +125,8 @@ pub async fn chunks_until_barrier(stream: impl MessageStream, expected_barrier:
124125
// `InternalMessage::Barrier(right_chunks, barrier)`.
125126
#[try_stream(ok = InternalMessage, error = StreamExecutorError)]
126127
async fn align_input(left: Box<dyn Executor>, right: Box<dyn Executor>) {
127-
let mut left = Box::pin(left.execute());
128-
let mut right = Box::pin(right.execute());
128+
let mut left = pin!(left.execute());
129+
let mut right = pin!(right.execute());
129130
// Keep producing intervals until stream exhaustion or errors.
130131
loop {
131132
let mut right_chunks = vec![];

0 commit comments

Comments
 (0)