-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fast Cache - Downgrade - reupgrade protection and other improvements #12
Changes from 15 commits
6350dc8
5f579c5
f6ebabb
89633f7
75d9f81
19ee7d3
cbf2f41
96208dc
5217e71
ea072a6
806b1a0
3f24e71
0cb480e
ee1ab84
2edb8cc
9712e87
eea8cfd
cd0a61c
8586d10
c293938
eae9425
2241204
31710b3
c0e4776
51b6c2f
a7e7fb2
f78896b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
package iavl | ||
|
||
import ( | ||
"bytes" | ||
"encoding/hex" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestFastNode_encodedSize(t *testing.T) { | ||
fastNode := &FastNode{ | ||
key: randBytes(10), | ||
versionLastUpdatedAt: 1, | ||
value: randBytes(20), | ||
} | ||
|
||
expectedSize := 1 + len(fastNode.value) + 1 | ||
|
||
require.Equal(t, expectedSize, fastNode.encodedSize()) | ||
} | ||
|
||
func TestFastNode_encode_decode(t *testing.T) { | ||
testcases := map[string]struct { | ||
node *FastNode | ||
expectHex string | ||
expectError bool | ||
}{ | ||
"nil": {nil, "", true}, | ||
"empty": {&FastNode{}, "0000", false}, | ||
"inner": {&FastNode{ | ||
key: []byte{0x4}, | ||
versionLastUpdatedAt: 1, | ||
value: []byte{0x2}, | ||
}, "020102", false}, | ||
} | ||
for name, tc := range testcases { | ||
tc := tc | ||
t.Run(name, func(t *testing.T) { | ||
var buf bytes.Buffer | ||
err := tc.node.writeBytes(&buf) | ||
if tc.expectError { | ||
require.Error(t, err) | ||
return | ||
} | ||
require.NoError(t, err) | ||
require.Equal(t, tc.expectHex, hex.EncodeToString(buf.Bytes())) | ||
|
||
node, err := DeserializeFastNode(tc.node.key, buf.Bytes()) | ||
require.NoError(t, err) | ||
// since value and leafHash are always decoded to []byte{} we augment the expected struct here | ||
if tc.node.value == nil { | ||
tc.node.value = []byte{} | ||
} | ||
require.Equal(t, tc.node, node) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,8 @@ import ( | |
"fmt" | ||
"math" | ||
"sort" | ||
"strconv" | ||
"strings" | ||
"sync" | ||
|
||
"github.com/pkg/errors" | ||
|
@@ -18,6 +20,12 @@ const ( | |
hashSize = sha256.Size | ||
genesisVersion = 1 | ||
storageVersionKey = "storage_version" | ||
// We store latest saved version together with storage version delimited by the constant below. | ||
// This delimiter is valid only if fast storage is enabled (i.e. storageVersion >= fastStorageVersionValue). | ||
// The latest saved version is needed for protection against downgrade and re-upgrade. In such a case, it would | ||
// be possible to observe mismatch between the latest version state and the fast nodes on disk. | ||
// Therefore, we would like to detect that and overwrite fast nodes on disk with the latest version state. | ||
fastStorageVersionDelimiter = "-" | ||
// Using semantic versioning: https://semver.org/ | ||
defaultStorageVersionValue = "1.0.0" | ||
fastStorageVersionValue = "1.1.0" | ||
|
@@ -211,6 +219,10 @@ func (ndb *nodeDB) SaveFastNode(node *FastNode) error { | |
} | ||
|
||
func (ndb *nodeDB) setStorageVersion(newVersion string) error { | ||
if newVersion >= fastStorageVersionValue { | ||
ValarDragon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
newVersion = newVersion + fastStorageVersionDelimiter + strconv.Itoa(int(ndb.latestVersion)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. won't we be infinitely appending to newVersion like this, if newVersion gets parsed from the serialized value later? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think any of the tests test repeated calls of parsing & setting but maybe i'm wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, good catch. I refactored this method completely and renamed it to |
||
} | ||
|
||
if err := ndb.db.Set(metadataKeyFormat.Key([]byte(storageVersionKey)), []byte(newVersion)); err != nil { | ||
return err | ||
} | ||
|
@@ -219,6 +231,10 @@ func (ndb *nodeDB) setStorageVersion(newVersion string) error { | |
} | ||
|
||
func (ndb *nodeDB) setStorageVersionBatch(newVersion string) error { | ||
if newVersion >= fastStorageVersionValue { | ||
newVersion = newVersion + fastStorageVersionDelimiter + strconv.Itoa(int(ndb.latestVersion)) | ||
} | ||
|
||
if err := ndb.batch.Set(metadataKeyFormat.Key([]byte(storageVersionKey)), []byte(newVersion)); err != nil { | ||
return err | ||
} | ||
|
@@ -234,6 +250,17 @@ func (ndb *nodeDB) isFastStorageEnabled() bool { | |
return ndb.getStorageVersion() >= fastStorageVersionValue | ||
} | ||
|
||
func (ndb *nodeDB) shouldForceFastStorageUpdate() bool { | ||
versions := strings.Split(ndb.storageVersion, fastStorageVersionDelimiter) | ||
|
||
if len(versions) == 2 { | ||
if versions[1] != strconv.Itoa(int(ndb.getLatestVersion())) { | ||
return true | ||
} | ||
} | ||
return false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should never happen under correct behavior right? Should we add a comment do a fmt.Println if this occurs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should happen when we have already upgraded to fast storage and it matches the latest state to avoid redundantly upgrading fast nodes when they are already live There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also happen if we are on the default (non-fast) storage version. This method is independent from To avoid confusion, hasUpgradedToFastStorage - returns true if we have upgraded to fast storage, false if we are on default shouldForceFastStorageUpdate - returns true if we have upgraded to fast storage but it does not match the live state, false otherwise |
||
} | ||
|
||
// SaveNode saves a FastNode to disk. | ||
func (ndb *nodeDB) saveFastNodeUnlocked(node *FastNode) error { | ||
if node.key == nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is removed because its only called if
tree.ndb.isFastStorageEnabled
is true?If so, can we add a comment stating this above the
saveFastNodeVersion
function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why this was removed is that now we are going with the assumption that fast cache storage will be upgraded to on all versions above 1.1.0 or fastStorageVersionValue. Therefore, we must make sure that we persist the latest live version on disk. Otherwise, re-upgrade protection would not function correctly.
In case there is any confusion, there are now 2 separate methods that check for fast storage to be enabled:
hasUpgradedToFastStorage
)isFastStorageEnabled
tohasUpgradedToFastStorage
and added a comment for clarityisFastStorageEnabled
)Let me know if this makes sense