hpides/viper

Consider persist `next_insert_pos` in varied-size VPage before return

Closed this issue · 2 comments

Hi,

The put function return without make next_insert_pos persistent. It may count on meta of each record to recover(the recover_database of varied_size value is not implemented).

viper/include/viper/viper.hpp

Lines 1137 to 1146 in e04d307

if (!is_inserted) {
// Entire record fits into current page.
entry.data = insert_pos + meta_size;
memcpy(entry.data, key.data(), entry.key_size);
memcpy(entry.data + entry.key_size, value.data(), entry.value_size);
internal::pmem_persist(entry.data, entry_length);
memcpy(insert_pos, &entry.size_info, meta_size);
internal::pmem_persist(insert_pos, meta_size);
v_page_->next_insert_pos = insert_pos + (meta_size + entry_length);
}

However, without next_insert_pos, we don't know how much space is used in a VPage because unused data in VPage is not always zeroed. I think it's better to use next_insert_pos as indicator of whether the new record is stored(just like free slot bitset for fixed-sized records). Then the code will be:

 if (!is_inserted) { 
     // Entire record fits into current page. 
     entry.data = insert_pos + meta_size; 
     memcpy(entry.data, key.data(), entry.key_size); 
     memcpy(entry.data + entry.key_size, value.data(), entry.value_size); 
     memcpy(insert_pos, &entry.size_info, meta_size); 
     pmem_persist(); 
     v_page_->next_insert_pos = insert_pos + (meta_size + entry_length); 
     pmem_persist(); 
 } 

Correct me if I am wrong. Thank you very much!

Thanks for opening these issues. I'm currently on vacation and will have a look at this and #10 in detail when I'm back in October.

I think you are correct about this. After every write to the next_insert_pos, this should also be persisted. During recovery, this can then be used as a reference as to how much space was used and how much data is valid. A positive side-effect of this change is that this removes the problem mentioned in #10. If a crash occurs while writing the size but the next_insert_pos is not yet updated, the record is not considered valid and will be discarded.

I'll try to submit a fix for this in the next few days.