moved max size check; added test for post-max size functioning

Signed-off-by: Matthew Sainsbury <[email protected]>

Author: Matthew Sainsbury

bolt_windows.go

@@ -68,19 +68,6 @@ func mmap(db *DB, sz int) error {
 	var sizelo, sizehi uint32
 
 	if !db.readOnly {
-		if db.MaxSize > 0 && sz > db.MaxSize {
-			// The max size only limits future writes; however, we don’t block opening
-			// and mapping the database if it already exceeds the limit.
-			fileSize, err := db.fileSize()
-			if err != nil {
-				return fmt.Errorf("could not check existing db file size: %s", err)
-			}
-
-			if sz > fileSize {
-				return errors.ErrMaxSizeReached
-			}
-		}
-
 		// Truncate the database to the size of the mmap.
 		if err := db.file.Truncate(int64(sz)); err != nil {
 			return fmt.Errorf("truncate: %s", err)

db.go

@@ -1166,13 +1166,26 @@ func (db *DB) allocate(txid common.Txid, count int) (*common.Page, error) {
 	p.SetId(db.rwtx.meta.Pgid())
 	var minsz = int((p.Id()+common.Pgid(count))+1) * db.pageSize
 	if minsz >= db.datasz {
-		if err := db.mmap(minsz); err != nil {
-			if err == berrors.ErrMaxSizeReached {
-				return nil, err
+		if !db.readOnly && db.MaxSize > 0 {
+			// this calculation matches the calculation in grow
+			// however, I don't quite understand it. Why is the allocation increment added to the size required,
+			// rather than the size required rounded up to the next multiple of the allocation increment?
+			nextAllocSize := minsz
+			if nextAllocSize < db.AllocSize {
+				nextAllocSize = db.AllocSize
 			} else {
-				return nil, fmt.Errorf("mmap allocate error: %s", err)
+				nextAllocSize += db.AllocSize
+			}
+
+			if nextAllocSize > db.MaxSize {
+				db.Logger().Errorf("[GOOS: %s, GOARCH: %s] maximum db size reached, size: %d, db.MaxSize: %d", runtime.GOOS, runtime.GOARCH, minsz, db.MaxSize)
+				return nil, berrors.ErrMaxSizeReached
 			}
 		}
+
+		if err := db.mmap(minsz); err != nil {
+			return nil, fmt.Errorf("mmap allocate error: %s", err)
+		}
 	}
 
 	// Move the page id high water mark.
@@ -1203,11 +1216,6 @@ func (db *DB) grow(sz int) error {
 		sz += db.AllocSize
 	}
 
-	if !db.readOnly && db.MaxSize > 0 && sz > db.MaxSize {
-		lg.Errorf("[GOOS: %s, GOARCH: %s] maximum db size reached, size: %d, db.MaxSize: %d", runtime.GOOS, runtime.GOARCH, sz, db.MaxSize)
-		return berrors.ErrMaxSizeReached
-	}
-
 	// Truncate and fsync to ensure file size metadata is flushed.
 	// https://github.com/boltdb/bolt/issues/284
 	if !db.NoGrowSync && !db.readOnly {

db_test.go

@@ -1491,6 +1491,14 @@ func fillDBWithKeys(db *btesting.DB, numKeys int) error {
 	)
 }
 
+// Convenience function for inserting a bunch of keys with values of a specific size (in bytes)
+func fillDBWithEntries(db *btesting.DB, numKeys int, valueSize int) error {
+	return db.Fill([]byte("data"), 1, numKeys,
+		func(tx int, k int) []byte { return []byte(fmt.Sprintf("%04d", k)) },
+		func(tx int, k int) []byte { return make([]byte, valueSize) },
+	)
+}
+
 // Creates a new database size, forces a specific allocation size jump, and fills it with the number of keys specified
 func createFilledDB(t testing.TB, o *bolt.Options, allocSize int, numKeys int) *btesting.DB {
 	// Open a data file.
@@ -1543,15 +1551,19 @@ func TestDB_MaxSizeNotExceeded(t *testing.T) {
 			path := db.Path()
 
 			// The data file should be 4 MiB now (expanded once from zero).
-			// It should have space for roughly 16 more entries before trying to grow
+			// It should have space for roughly one more entry with value size 100kB before trying to grow
 			// Keep inserting until grow is required
-			err := fillDBWithKeys(db, 100)
+			err := fillDBWithEntries(db, 2, 100000)
 			assert.ErrorIs(t, err, berrors.ErrMaxSizeReached)
 
 			newSz := fileSize(path)
 			require.Greater(t, newSz, int64(0), "unexpected new file size: %d", newSz)
 			assert.LessOrEqual(t, newSz, int64(db.MaxSize), "The size of the data file should not exceed db.MaxSize")
 
+			// Now try another write that shouldn't increase the max size
+			err = fillDBWithEntries(db, 1, 1)
+			assert.NoError(t, err, "Adding an entry after a failed, oversized write should not error")
+
 			err = db.Close()
 			require.NoError(t, err, "Closing the re-opened database should succeed")
 		})