risingwave icon indicating copy to clipboard operation
risingwave copied to clipboard

build(toolchain): bump to 2022-09-22

Open xxchan opened this issue 3 years ago • 1 comments

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

xxchan avatar Sep 05 '22 23:09 xxchan

The ICE is fixed. Now we have TAIT bug 🤪 https://github.com/rust-lang/rust/issues/101750

xxchan avatar Sep 13 '22 08:09 xxchan

Really want https://github.com/rust-lang/cargo/pull/11114 after bumping. 🤤

BugenZhao avatar Oct 08 '22 05:10 BugenZhao

Build of risingwave_stream will stuck on macOS. Still investigating why.

skyzh avatar Oct 17 '22 00:10 skyzh

INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::mir::ConstantKind>: result=Ok(Val(ZeroSized, for<'a, 'b, 'c> fn(std::pin::Pin<&'a mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>>, &'b mut std::task::Context<'c>) -> std::task::Poll<std::option::Option<<futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]> as tokio_stream::Stream>::Item>> {<futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]> as tokio_stream::Stream>::poll_next})) with 4052 obligations
INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::ty::subst::GenericArg>: result=Ok(unsafe fn(&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>) -> std::pin::Pin<&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>> {std::pin::Pin::<&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>>::new_unchecked}) with 4052 obligations
INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::mir::ConstantKind>: result=Ok(Val(ZeroSized, unsafe fn(&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>) -> std::pin::Pin<&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>> {std::pin::Pin::<&mut futures_async_stream::try_stream::from_generator::GenTryStream<[static generator@src/stream/src/executor/hash_join.rs:760:5: 760:61]>>::new_unchecked})) with 4052 obligations

It seems that Rust is very slow with resolving futures_async_stream. It took about 1sec for each resolving.

skyzh avatar Oct 17 '22 00:10 skyzh

To reproduce, RUSTC_LOG=debug ./risedev d

skyzh avatar Oct 17 '22 00:10 skyzh

Seems there's some new false positives for resolving lifetime failure. 🤣

BugenZhao avatar Oct 17 '22 00:10 BugenZhao

As a comparison, previous builds only have 0 obligations for all these things.

skyzh avatar Oct 17 '22 00:10 skyzh

future_async_stream requires Generator, which seems to have performance issues. Probably related to https://github.com/rust-lang/rust/pull/101692. I can test if that PR would solve this issue by manually build a Rust compiler with that branch...

skyzh avatar Oct 17 '22 01:10 skyzh

Also the lifetime issue doesn't look like a false positive.

        let rotate_last_mut = |buffers: &'a mut Vec<_>| {
            buffers.push(DioBuffer::with_capacity_in(
                self.buffer_capacity,
                &DIO_BUFFER_ALLOCATOR,
            ));
            buffers.last_mut().unwrap()
        };

is defined for 'a mut Vec

where

rotate_last_mut(&mut self.buffers),

is following the lifetime of &mut self. buffers' lifetime has no relationship to 'a. The compiler's report seems correct.

cc @MrCroxx would you please help fix this?

skyzh avatar Oct 17 '22 01:10 skyzh

Unluckily, https://github.com/rust-lang/rust/pull/101692 doesn't help much 🤣 Probably we will need to cooperate with the Rust community to fix this...

  • futures_async_stream seems to generate too much more obligations than before.
  • need fix lifetime issues
  • may merge this PR without bumping the toolchain

skyzh avatar Oct 17 '22 02:10 skyzh

Okay, the problem comes to HashJoin again. After clearing all function body of HashJoin:

#[try_stream(ok = Message, error = StreamExecutorError)]
async fn into_stream(mut self) {}

Everything works!

@yuhao-su are you still maintaining the streaming hash join component? Do you have time to look into why HashJoin is causing rustc to stuck?

skyzh avatar Oct 17 '22 02:10 skyzh

Do you have time to look into why HashJoin is causing rustc to stuck?

I'll take a look.

yuhao-su avatar Oct 17 '22 02:10 yuhao-su

Unluckily, the bombing obligations seem to originate from StateTableIter

INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::ty::subst::GenericArg>: result=Ok(std::future::from_generator::GenFuture<[static generator@src/stream/src/common/mod.rs:31:45: 37:2]>) with 4052 obligations
INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::ty::subst::GenericArg>: result=Ok(std::future::from_generator::GenFuture<[static generator@risingwave_storage::table::streaming_table::state_table::StateTable<risingwave_storage::monitor::MonitoredStateStore<risingwave_storage::hummock::HummockStorage>>::iter::{closure#0}]>) with 4052 obligations
INFO rustc_trait_selection::traits::query::normalize normalize::<rustc_middle::ty::subst::GenericArg>: result=Ok(std::future::from_generator::GenFuture<[static generator@risingwave_storage::table::streaming_table::state_table::StateTable<risingwave_storage::monitor::MonitoredStateStore<risingwave_storage::hummock::HummockStorage>>::iter_with_pk_prefix::{closure#0}]>) with 4052 obligations

which cases all executors to have a lot of obligations, whereas hash join needs a lot of specialization, and cases the most problems.

skyzh avatar Oct 18 '22 01:10 skyzh

After changing iter_key_and_val to boxed, risingwave_stream compiles in 40 seconds (still seems slow). cc @BugenZhao now it's your turn to investigate what's happening 🥵

    pub async fn iter_key_and_val<'a>(
        &'a self,
        pk_prefix: &'a Row,
    ) -> StorageResult<RowStreamWithPk<'a, S>> {
        let (mem_table_iter, storage_iter_stream) = self
            .iter_with_pk_prefix_inner(pk_prefix, self.epoch())
            .await?;
        let storage_iter = storage_iter_stream.into_stream().boxed();

        Ok(
            StateTableRowIter::new(mem_table_iter, storage_iter, self.row_deserializer.clone())
                .into_stream().boxed(),
        )
    }

skyzh avatar Oct 18 '22 01:10 skyzh

My wild guess is that while rustc allows more lifetime generics in the latest version, some wrongly-labeled lifetimes will cause compile issues. Every time we add 'a at &self, there must be something going wrong.

skyzh avatar Oct 18 '22 01:10 skyzh

The issue now narrows down to risingwave_storage. It seems that it produces 4052 obligations...

image

Most of them are OutlivesPredicate (lifetime issues)

skyzh avatar Oct 18 '22 01:10 skyzh

turn to @wenym1

TennyZhuang avatar Oct 18 '22 01:10 TennyZhuang

IIRC storage have many unnecessary generics, but I’m not sure whether it’s related.

TennyZhuang avatar Oct 18 '22 01:10 TennyZhuang

I feel this issue more related to StateTable implementation than HummockStorage itself. The obligations will only bomb when processing StateTable-related code.

skyzh avatar Oct 18 '22 02:10 skyzh

Will implementing state table iterator manually with poll_next help?

BugenZhao avatar Oct 18 '22 02:10 BugenZhao

I'm not sure, but I'd suggest investigating where's the obligations from.

skyzh avatar Oct 18 '22 02:10 skyzh

Also the lifetime issue doesn't look like a false positive.

        let rotate_last_mut = |buffers: &'a mut Vec<_>| {
            buffers.push(DioBuffer::with_capacity_in(
                self.buffer_capacity,
                &DIO_BUFFER_ALLOCATOR,
            ));
            buffers.last_mut().unwrap()
        };

is defined for 'a mut Vec

where

rotate_last_mut(&mut self.buffers),

is following the lifetime of &mut self. buffers' lifetime has no relationship to 'a. The compiler's report seems correct.

cc @MrCroxx would you please help fix this?

Sure. I'll handle it.

MrCroxx avatar Oct 19 '22 01:10 MrCroxx

https://github.com/risingwavelabs/risingwave/issues/1334 can be closed after this

lmatz avatar Oct 24 '22 05:10 lmatz

I'll take over this PR and get it merged with my pace.

skyzh avatar Oct 25 '22 03:10 skyzh

Thank you guys very much

vickiegpt avatar Oct 26 '22 23:10 vickiegpt

close in favor of https://github.com/risingwavelabs/risingwave/pull/6025

skyzh avatar Oct 27 '22 21:10 skyzh