client-rust icon indicating copy to clipboard operation
client-rust copied to clipboard

prevent PANIC when leader is None

Open cre4ture opened this issue 1 year ago • 3 comments

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

cre4ture avatar May 22 '24 18:05 cre4ture

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 ~

pingyu avatar Jun 02 '24 15:06 pingyu

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?

cre4ture avatar Jun 04 '24 10:06 cre4ture

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.

pingyu avatar Jun 04 '24 11:06 pingyu