Avoid using Get interface to obtain metadata when iterating data during migration

[caipengbo]

Fixed #904

[datavisorxiaobiaozhao]

The problem we had was that when we migrated, we had a lot of disk reads, but with this optimized version, these abnormal disk reads disappeared, so I think GetMetaData calls the Get interface

[caipengbo]

The problem we had was that when we migrated, we had a lot of disk reads, but with this optimized version, these abnormal disk reads disappeared, so I think GetMetaData calls the Get interface.

@datavisorxiaobiaozhao Yes. During the iteration, we set the cache not to be filled:

https://github.com/apache/incubator-kvrocks/blob/e8721a270d23345f385af6db0617c3549d6e6812/src/slot_migrate.cc#L273-L277

so that when we call DB::Get(), we are likely to read the disk.

[shangxiaoxiong]

There may be some problem. The Get interface may aim to get the latest expire time of the key.

[caipengbo]

There may be some problem. The Get interface may aim to get the latest expire time of the key.

@shangxiaoxiong Oh Yes, I missed that. Then we only need GetMetaData for those with expiration time. Or we can record the time snapshot was created and subtract the expiration time to determine whether it is expired!

[shangxiaoxiong]

During sending snapshot, exipre time of some key may be changed to larger by clients. GetMetaData is needed to deal with this case.

[shangxiaoxiong]

l think GetMetaData is needed for complex key with expire time to get the latest expire time.

[caipengbo]

During sending snapshot, exipre time of some key may be changed to larger by clients. GetMetaData is needed to deal with this case.

During sending snapshot:

• for simple type, user changes to expire are incremental data and will be synchronized through WAL.
• for complex types, we really should get the latest metadata.

[shangxiaoxiong]

During sending snapshot, exipre time of some key may be changed to larger by clients. GetMetaData is needed to deal with this case.

During sending snapshot, user changes to expire are incremental data and will be synchronized through WAL.

Simple key is ok but complex key is not.

[git-hulk]

During sending snapshot, exipre time of some key may be changed to larger by clients. GetMetaData is needed to deal with this case.

During sending snapshot, user changes to expire are incremental data and will be synchronized through WAL.

Simple key is ok but complex key is not.

It can't resolve this issue even we use the latest snapshot since the change may come after.

[shangxiaoxiong]

Yeah. Using GetMetaData with latest snapshot can't solve the problem completely. Just a little improvement.

[caipengbo]

Indeed, the whole process is not atomic.

[git-hulk]

Yes, the root cause is the snapshot can be different between writers and background threads. I prefer to use current implementation, then we can try to seek another way to resolve this issue completely. How do you think? @caipengbo @shangxiaoxiong

[shangxiaoxiong]

Current implementation makes slotmigration faster. Faster slotmigration is, smaller risk of losing key is. On the whole, current implementation may be good.

[caipengbo]

I think OK, I will continue to think about how to solve this kind of problem.

[xiaobiaozhao]

I think this is the best solution for now, merge this pr first, and then contionue to look for new solutions.

[caipengbo]

Thanks all for the great discussion, I'll explain the problems with this implementation in #904