search

tikv / client-rust

Rust Client for TiKV.
Apache License 2.0
389 stars 131 forks source link

prevent PANIC when leader is None #450

Closed cre4ture closed 2 months ago

cre4ture commented 6 months ago

Hello,

I'm currently experimenting with TiFS (https://github.com/Hexilee/tifs). When copying large files, the TiKV cluster that I have locally gets under a constant load for a longer period of time (~1-2 hours). During these tests I came accross a panic caused by the rust-client. I located the code responsible for this, and implemented a fix. Please have a look, tell me what you think and what to adapt to let it get into the main branch.

Thanks and best regards, cre4ture

log entry of the panic:

thread 'tokio-runtime-worker' panicked at /home/uli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tikv-client-0.3.0/src/pd/cluster.rs:270:47: called `Option::unwrap()` on a `None` value
pingyu commented 6 months ago

The issue of handling get_members of PD was fixed by #452. We should not encounter the situation that leader is None anymore.

@cre4ture Could you help with some testing to confirm that this issue has been completely resolved ? Thank you ~

cre4ture commented 5 months ago

The issue of handling get_members of PD was fixed by #452. We should not encounter the situation that leader is None anymore.

@cre4ture Could you help with some testing to confirm that this issue has been completely resolved ? Thank you ~

I will test and report in case I still see it. Thanks for notification.

I assume this PR would be obsolete then? Or should I prepare still some parts of it for merging?

pingyu commented 5 months ago

The issue of handling get_members of PD was fixed by #452. We should not encounter the situation that leader is None anymore. @cre4ture Could you help with some testing to confirm that this issue has been completely resolved ? Thank you ~

I will test and report in case I still see it. Thanks for notification.

I assume this PR would be obsolete then? Or should I prepare still some parts of it for merging?

It is obsolete now. Unless it's not the reason of get_members in your case.