From 4c153a8b365b51110b23244bb4ebe6cc4e86256c Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 16 Aug 2019 16:59:44 -0700 Subject: [PATCH 1/3] Fix bug in backup KeyToList function. Previously, the KeyToList function used during backup assumed that ReadPostingList takes you to the next key so calling iterator.Next manually is not needed. This is not the case. The fix is either to make ReadPostingList call iterator.Next before returning or doing it in the KeyToList function. The latter has been chosen in this PR to preserve the existing behavior of ReadPostingList. It's still not clear why only the bulk loader triggered this issue. --- ee/backup/backup.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ee/backup/backup.go b/ee/backup/backup.go index f1e3d571959..2c1693e3dda 100644 --- a/ee/backup/backup.go +++ b/ee/backup/backup.go @@ -264,15 +264,12 @@ func (pr *Processor) toBackupList(key []byte, itr *badger.Iterator) (*bpb.KVList return list, nil } - // Manually advance the iterator. This cannot be done in the for - // statement because ReadPostingList advances the iterator so this - // only needs to be done for BitSchemaPosting entries. - itr.Next() - default: return nil, errors.Errorf( "Unexpected meta: %d for key: %s", item.UserMeta(), hex.Dump(key)) } + + itr.Next() } // This shouldn't be reached but it's being added here because the golang From 11a84b90c9246063dbf650d231560351f2d0d43b Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 16 Aug 2019 23:48:38 -0700 Subject: [PATCH 2/3] Call it.Next inside ReadPostingList instead. --- ee/backup/backup.go | 3 +-- posting/mvcc.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ee/backup/backup.go b/ee/backup/backup.go index 2c1693e3dda..26ee4191177 100644 --- a/ee/backup/backup.go +++ b/ee/backup/backup.go @@ -263,13 +263,12 @@ func (pr *Processor) toBackupList(key []byte, itr *badger.Iterator) (*bpb.KVList if item.DiscardEarlierVersions() || item.IsDeletedOrExpired() { return list, nil } + itr.Next() default: return nil, errors.Errorf( "Unexpected meta: %d for key: %s", item.UserMeta(), hex.Dump(key)) } - - itr.Next() } // This shouldn't be reached but it's being added here because the golang diff --git a/posting/mvcc.go b/posting/mvcc.go index 6f46e8b57e7..bfb015e5447 100644 --- a/posting/mvcc.go +++ b/posting/mvcc.go @@ -167,8 +167,7 @@ func ReadPostingList(key []byte, it *badger.Iterator) (*List, error) { return nil, err } l.minTs = item.Version() - // No need to do Next here. The outer loop can take care of skipping - // more versions of the same key. + it.Next() return l, nil case BitDeltaPosting: err := item.Value(func(val []byte) error { From b221172c506e16acdfe84d9beb4107622593c291 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Sat, 17 Aug 2019 00:15:40 -0700 Subject: [PATCH 3/3] Restore comment. --- ee/backup/backup.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ee/backup/backup.go b/ee/backup/backup.go index 26ee4191177..f1e3d571959 100644 --- a/ee/backup/backup.go +++ b/ee/backup/backup.go @@ -263,6 +263,10 @@ func (pr *Processor) toBackupList(key []byte, itr *badger.Iterator) (*bpb.KVList if item.DiscardEarlierVersions() || item.IsDeletedOrExpired() { return list, nil } + + // Manually advance the iterator. This cannot be done in the for + // statement because ReadPostingList advances the iterator so this + // only needs to be done for BitSchemaPosting entries. itr.Next() default: