Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Thanks @sihuazhou ! LGTM ð Will merge this.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
cc @StefanRRichter
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Let's wait a bit more for their response. It seems like this example is
older than their corrected docs.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Have't received any response from RocksDB yet, but I found this example
with using `RocksIterator#stats()`:
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
FYI, I found this issue related to problem:
https://github.com/facebook/rocksdb/issues/3558
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Agreed!
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Maybe we should ask them on their issue tracker what the best practise is?
I cannot remember seeing such checks in their code examples. Have a hard time
to believe that this can be true,
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
It depends, maybe this is already covered currently because we might always
do an iteration attempt that checks right after the seek. But in general, this
is not very nice and fragile if
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Oh My God...Is that means we need to wrap the `RocksIterator` to delegate
all it API?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
After double checking with the RocksDB docs, I am afraid that we need to
introduce more checks because for example the point out that also after methods
like `seek` the iterator an become
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Agree, should be correct first before fast! Could you please have a look at
this? I think it's already for a look now~
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
It sounds cheap if they just `&` all the flags from the sub iterators. In
the end, we can see if there is a performance drop but better be correct first
before fast.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter I had a look at the implementation of the iterators in
RocksDB, I found status just return the flag first `_status` as the result
without any complex computation, But for some
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I'm going to check the native implementation and see whether the `status()`
is a super cheap option...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I think I am a bit torn here now...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
That is a good question, and I'm not sure...but I think that seems to be...
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Oh you are right, this is confusing :-) So does this also mean the status
flag is cleared when we simple continue iterating and only check in the end?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I think that is the incorrect one, If I'm not confused by the wiki's
content...
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Also from the RocksDB docs: `In another word, if Iterator::Valid() is true,
status() is guaranteed to be OK()`
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Yes, but eventually it will also return `false`, which is essentially the
same as waiting until the loop terminates. Anyways, I think after the loop is
the nicer way.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter NO, I think that couldn't fix this issue, the problem here
is that even `iterator.isValid()` return `true`, there may also some internal
error in RocksDB. What do you think?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
You could also in `isRocksIteratorValid` run the check only if the return
value is `false` if you like the helper method to avoid people forgetting about
this check.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter No, I didn't have any performance tests yet. I think you
are right! Your proposal is the way I'm going to choose. Addressing this...
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
A quick general question: could you observe any performance impact from
calling the `status()` method in the loops. It looks like a native method and I
am not sure that it is inexpensive.
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
The reasons that the travis given red light is unrelated...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
cc @StefanRRichter
---
26 matches
Mail list logo
