Repository: dgraph-io/badger
Pull request #1400 (2020-07-08 11:43:09)

return error if the vlog writes exceeds more that 4GB.
This PR check whether vlog writes are bigger than 4GB. If it's bigger than 4GB, it returns an error.
Signed-off-by: பாலாஜி <balaji@dgraph.io>

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/dgraph-io/badger/1400)
<!-- Reviewable:end -->



Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0000000..30512e9
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,107 @@
+# Contribution Guide
+
+* [Before you get started](#before-you-get-started)
+    * [Code of Conduct](#code-of-conduct)
+* [Your First Contribution](#your-first-contribution)
+    * [Find a good first topic](#find-a-good-first-topic)
+* [Setting up your development environment](#setting-up-your-development-environment)
+    * [Fork the project](#fork-the-project)
+    * [Clone the project](#clone-the-project)
+    * [New branch for a new code](#new-branch-for-a-new-code)
+    * [Test](#test)
+    * [Commit and push](#commit-and-push)
+    * [Create a Pull Request](#create-a-pull-request)
+    * [Sign the CLA](#sign-the-cla)
+    * [Get a code review](#get-a-code-review)
+
+## Before you get started
+
+### Code of Conduct
+
+Please make sure to read and observe our [Code of Conduct](./CODE_OF_CONDUCT.md).
+
+## Your First Contribution
+
+### Find a good first topic
+
+You can start by finding an existing issue with the
+[good first issue](https://github.com/dgraph-io/badger/labels/good%20first%20issue) or [help wanted](https://github.com/dgraph-io/badger/labels/help%20wanted) labels. These issues are well suited for new contributors.
+
+
+## Setting up your development environment
+
+Badger uses [`Go Modules`](https://github.com/golang/go/wiki/Modules)
+to manage dependencies. The version of Go should be **1.12** or above.
+
+### Fork the project
+
+- Visit https://github.com/dgraph-io/badger
+- Click the `Fork` button (top right) to create a fork of the repository
+
+### Clone the project
+
+```sh
+$ git clone https://github.com/$GITHUB_USER/badger
+$ cd badger
+$ git remote add upstream git@github.com:dgraph-io/badger.git
+
+# Never push to the upstream master
+git remote set-url --push upstream no_push
+```
+
+### New branch for a new code
+
+Get your local master up to date:
+
+```sh
+$ git fetch upstream
+$ git checkout master
+$ git rebase upstream/master
+```
+
+Create a new branch from the master:
+
+```sh
+$ git checkout -b my_new_feature
+```
+
+And now you can finally add your changes to project.
+
+### Test
+
+Build and run all tests:
+
+```sh
+$ ./test.sh
+```
+
+### Commit and push
+
+Commit your changes:
+
+```sh
+$ git commit
+```
+
+When the changes are ready to review:
+
+```sh
+$ git push origin my_new_feature
+```
+
+### Create a Pull Request
+
+Just open `https://github.com/$GITHUB_USER/badger/pull/new/my_new_feature` and
+fill the PR description.
+
+### Sign the CLA
+
+Click the **Sign in with Github to agree** button to sign the CLA. [An example](https://cla-assistant.io/dgraph-io/badger?pullRequest=1377).
+
+### Get a code review
+
+If your pull request (PR) is opened, it will be assigned to one or more
+reviewers. Those reviewers will do a code review.
+
+To address review comments, you should commit the changes to the same branch of
+the PR on your fork.
diff --git a/README.md b/README.md
index ab0a03f..2a22091 100644
--- a/README.md
+++ b/README.md
@@ -59,6 +59,7 @@ For more details on our version naming schema please read [Choosing a version](#
     + [Comparisons](#comparisons)
     + [Benchmarks](#benchmarks)
   * [Projects Using Badger](#projects-using-badger)
+  * [Contributing](#contributing)
   * [Frequently Asked Questions](#frequently-asked-questions)
 
 ## Getting Started
@@ -787,6 +788,10 @@ Below is a list of known projects that use Badger:
 
 If you are using Badger in a project please send a pull request to add it to the list.
 
+## Contributing
+
+If you're interested in contributing to Badger see [CONTRIBUTING.md](./CONTRIBUTING.md).
+
 ## Frequently Asked Questions
 ### My writes are getting stuck. Why?
 
diff --git a/table/builder.go b/table/builder.go
index 50ba153..6a6f194 100644
--- a/table/builder.go
+++ b/table/builder.go
@@ -327,6 +327,9 @@ func (b *Builder) shouldFinishBlock(key []byte, value y.ValueStruct) bool {
 		// So, size of IV is added to estimatedSize.
 		estimatedSize += aes.BlockSize
 	}
+	// Integer overflow check for table size.
+	y.AssertTrue(uint64(b.sz)+uint64(estimatedSize) < math.MaxUint32)
+
 	return estimatedSize > uint32(b.opt.BlockSize)
 }
 
diff --git a/value.go b/value.go
index e675075..75bb8b4 100644
--- a/value.go
+++ b/value.go
@@ -45,6 +45,10 @@ import (
 	"golang.org/x/net/trace"
 )
 
+// maxVlogFileSize is the maximum size of the vlog file which can be created. Vlog Offset is of
+// uint32, so limiting at max uint32.
+var maxVlogFileSize = math.MaxUint32
+
 // Values have their first byte being byteData or byteDelete. This helps us distinguish between
 // a key that has never been seen and a key that has been explicitly deleted.
 const (
@@ -1364,11 +1368,52 @@ func (vlog *valueLog) woffset() uint32 {
 	return atomic.LoadUint32(&vlog.writableLogOffset)
 }
 
+// validateWrites will check whether the given requests can fit into 4GB vlog file.
+// NOTE: 4GB is the maximum size we can create for vlog because value pointer offset is of type
+// uint32. If we create more than 4GB, it will overflow uint32. So, limiting the size to 4GB.
+func (vlog *valueLog) validateWrites(reqs []*request) error {
+	vlogOffset := uint64(vlog.woffset())
+	for _, req := range reqs {
+		// calculate size of the request.
+		size := estimateRequestSize(req)
+		estimatedVlogOffset := vlogOffset + size
+		if estimatedVlogOffset > uint64(maxVlogFileSize) {
+			return errors.Errorf("Request size offset %d is bigger than maximum offset %d",
+				estimatedVlogOffset, maxVlogFileSize)
+		}
+
+		if estimatedVlogOffset >= uint64(vlog.opt.ValueLogFileSize) {
+			// We'll create a new vlog file if the estimated offset is greater or equal to
+			// max vlog size. So, resetting the vlogOffset.
+			vlogOffset = 0
+			continue
+		}
+		// Estimated vlog offset will become current vlog offset if the vlog is not rotated.
+		vlogOffset = estimatedVlogOffset
+	}
+	return nil
+}
+
+// estimateRequestSize returns the size that needed to be written for the given request.
+func estimateRequestSize(req *request) uint64 {
+	size := uint64(0)
+	for _, e := range req.Entries {
+		size += uint64(maxHeaderSize + len(e.Key) + len(e.Value) + crc32.Size)
+	}
+	return size
+}
+
 // write is thread-unsafe by design and should not be called concurrently.
 func (vlog *valueLog) write(reqs []*request) error {
 	if vlog.db.opt.InMemory {
 		return nil
 	}
+	// Validate writes before writing to vlog. Because, we don't want to partially write and return
+	// an error.
+	if err := vlog.validateWrites(reqs); err != nil {
+		return err
+	}
+
 	vlog.filesLock.RLock()
 	maxFid := vlog.maxFid
 	curlf := vlog.filesMap[maxFid]
diff --git a/y/y.go b/y/y.go
index 1412223..5e6dd5b 100644
--- a/y/y.go
+++ b/y/y.go
@@ -191,8 +191,9 @@ func FixedDuration(d time.Duration) string {
 // to tell the goroutine to shut down, and a WaitGroup with which to wait for it to finish shutting
 // down.
 type Closer struct {
-	closed  chan struct{}
-	waiting sync.WaitGroup
+	closed    chan struct{}
+	waiting   sync.WaitGroup
+	closeOnce sync.Once
 }
 
 // NewCloser constructs a new Closer, with an initial count on the WaitGroup.
@@ -209,7 +210,10 @@ func (lc *Closer) AddRunning(delta int) {
 
 // Signal signals the HasBeenClosed signal.
 func (lc *Closer) Signal() {
-	close(lc.closed)
+	// Todo(ibrahim): Change Signal to return error on next badger breaking change.
+	lc.closeOnce.Do(func() {
+		close(lc.closed)
+	})
 }
 
 // HasBeenClosed gets signaled when Signal() is called.


Test patch:
diff --git a/value_test.go b/value_test.go
index fd5b9b0..f08348c 100644
--- a/value_test.go
+++ b/value_test.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"math/rand"
 	"os"
 	"reflect"
@@ -1237,3 +1238,64 @@ func TestValueEntryChecksum(t *testing.T) {
 		require.NoError(t, db.Close())
 	})
 }
+
+func TestValidateWrite(t *testing.T) {
+	// Mocking the file size, so that we don't allocate big memory while running test.
+	maxVlogFileSize = 400
+	defer func() {
+		maxVlogFileSize = math.MaxUint32
+	}()
+
+	bigBuf := make([]byte, maxVlogFileSize+1)
+	log := &valueLog{
+		opt: DefaultOptions("."),
+	}
+
+	// Sending a request with big values which will overflow uint32.
+	key := []byte("HelloKey")
+	req := &request{
+		Entries: []*Entry{
+			{
+				Key:   key,
+				Value: bigBuf,
+			},
+			{
+				Key:   key,
+				Value: bigBuf,
+			},
+			{
+				Key:   key,
+				Value: bigBuf,
+			},
+		},
+	}
+
+	err := log.validateWrites([]*request{req})
+	require.Error(t, err)
+
+	// Testing with small values.
+	smallBuf := make([]byte, 4)
+	req1 := &request{
+		Entries: []*Entry{
+			{
+				Key:   key,
+				Value: smallBuf,
+			},
+			{
+				Key:   key,
+				Value: smallBuf,
+			},
+			{
+				Key:   key,
+				Value: smallBuf,
+			},
+		},
+	}
+
+	err = log.validateWrites([]*request{req1})
+	require.NoError(t, err)
+
+	// Batching small and big request.
+	err = log.validateWrites([]*request{req1, req})
+	require.Error(t, err)
+}
diff --git a/y/y_test.go b/y/y_test.go
index d1b9631..fe14cf4 100644
--- a/y/y_test.go
+++ b/y/y_test.go
@@ -254,3 +254,20 @@ func TestPagebufferReader4(t *testing.T) {
 	require.Equal(t, err, io.EOF, "should return EOF")
 	require.Equal(t, n, 0)
 }
+
+func TestMulipleSignals(t *testing.T) {
+	closer := NewCloser(0)
+	require.NotPanics(t, func() { closer.Signal() })
+	// Should not panic.
+	require.NotPanics(t, func() { closer.Signal() })
+	require.NotPanics(t, func() { closer.SignalAndWait() })
+
+	// Attempt 2.
+	closer = NewCloser(1)
+	require.NotPanics(t, func() { closer.Done() })
+
+	require.NotPanics(t, func() { closer.SignalAndWait() })
+	// Should not panic.
+	require.NotPanics(t, func() { closer.SignalAndWait() })
+	require.NotPanics(t, func() { closer.Signal() })
+}