[BUG] incorrect level information of hit file in Version::Get()

[cighao]

In Version::Get(), once hit a file, calls

table_cache_->Get(
        read_options, *internal_comparator(), *f->file_metadata, ikey,
        &get_context, mutable_cf_options_.prefix_extractor.get(),
        cfd_->internal_stats()->GetFileReadHist(fp.GetHitFileLevel()),
        IsFilterSkipped(static_cast<int>(fp.GetHitFileLevel()),
       fp.IsHitFileLastInLevel()),
       fp.GetCurrentLevel());

to get this file.

fp.GetCurrentLevel() should refer to which level this file belongs to. But actually fp.GetCurrentLevel() does not represent the level that the file belongs to.

In fp.GetNextFile(),

...
returned_file_level_ = curr_level_;
if (curr_level_ > 0 && cmp_largest < 0) {
     // No more files to search in this level.
     search_ended_ = !PrepareNextLevel();  // increase curr_level_ in PrepareNextLevel()
 } else {
      ++curr_index_in_curr_level_;
}
return f;
...

if hit a file, it firstly calls PrepareNextLevel(), which will increase curr_level_, then return f. After returning f, the curr_level_ does not refer to which level f belongs to, and it points to the next level.

I think it should change fp.GetCurrentLevel() to fp.GetHitFileLevel() in

table_cache_->Get(
        read_options, *internal_comparator(), *f->file_metadata, ikey,
        &get_context, mutable_cf_options_.prefix_extractor.get(),
        cfd_->internal_stats()->GetFileReadHist(fp.GetHitFileLevel()),
        IsFilterSkipped(static_cast<int>(fp.GetHitFileLevel()),
       fp.IsHitFileLastInLevel()),
       fp.GetCurrentLevel());

[ajkr]

These static_cast<int>(which)es are also wrong:

rocksdb/db/version_set.cc

Lines 5675 to 5689 in 14eca6b

	/*skip_filters=*/false, /*level=*/static_cast<int>(which),
	/*smallest_compaction_key=*/nullptr,
	/*largest_compaction_key=*/nullptr,
	/*allow_unprepared_value=*/false);
	}
	} else {
	// Create concatenating iterator for the files from this level
	list[num++] = new LevelIterator(
	cfd->table_cache(), read_options, file_options_compactions,
	cfd->internal_comparator(), c->input_levels(which),
	c->mutable_cf_options()->prefix_extractor.get(),
	/*should_sample=*/false,
	/*no per level latency histogram=*/nullptr,
	TableReaderCaller::kCompaction, /*skip_filters=*/false,
	/*level=*/static_cast<int>(which), range_del_agg,

. The implication of these and the one you mentioned looks like prefetching/pinning can apply to the "wrong" files. It's more an optimization problem than correctness. If you're interested in a fix that'd be really useful -- the level parameter may be used for other things one day, so hopefully we can trust it is correct..

edit: Oh, I just realized you also reported the other problems in #6667.