Skip to content
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

Disable cache by default #1257

Merged
merged 6 commits into from
Mar 16, 2020
Merged

Disable cache by default #1257

merged 6 commits into from
Mar 16, 2020

Conversation

jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Mar 12, 2020

Disabling cache by default leads to better performance when compression
and encryption is disabled.

Result of read bench tool with key size 32 bytes and value size 128 bytes
(Note: data is not compressed or encrypted)

1. With 1 GB cache:
       	Average read speed              : 41.3833 MB/s
      	Total bytes read in 1 minute    : 2.4 GB

2. Without cache:
	Average read speed		: 58.86 MB/s
	Total bytes read in 1 minute	: 3.3 GB

This change is Reviewable

Ibrahim Jarif added 2 commits March 12, 2020 17:30
This reverts commit 4676ca9.

Reading bloom filters from the cache leads to severe preformance
degradation.
Result of read bench tool
1. With bloom filters in cache
	Average read speed		: 97.37 KB/s
	Total bytes read in 1 minute	: 3.7 MB

2. With bloom filters stored in memory (no cache)
	Average read speed		: 41.3833 MB/s
	Total bytes read in 1 minute	: 2.4 GB
Disabling cache by default leads to better perfomance when compressoin
and encryption are disabled.

Result of read bench tool (data is not encrypted or compressed)
1. With 1 GB cache:
       	Average read speed              : 41.3833 MB/s
      	Total bytes read in 1 minute    : 2.4 GB

2. Without cache:
	Average read speed		: 58.86 MB/s
	Total bytes read in 1 minute	: 3.3 GB
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @jarifibrahim)


table/builder_test.go, line 60 at r1 (raw file):

		for _, opt := range opts {
			builder := NewTableBuilder(opt)
			filename := fmt.Sprintf("%s%c%d.sst", os.TempDir(), os.PathSeparator, rand.Int63())

Int63 can return a negative number?

Copy link
Contributor Author

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)


table/builder_test.go, line 60 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Int63 can return a negative number?

rand.Int63() does not return a negative number

Int63 returns a non-negative pseudo-random 63-bit integer as an int64.

https://golang.org/pkg/math/rand/#Rand.Int63

@jarifibrahim jarifibrahim merged commit 91c31eb into master Mar 16, 2020
@jarifibrahim jarifibrahim deleted the ibrahim/disable-cache branch March 16, 2020 17:56
jarifibrahim pushed a commit that referenced this pull request Mar 24, 2020
Disabling cache by default leads to better performance when compression
and encryption is disabled.

Result of read bench tool (data is not encrypted or compressed)
1. With 1 GB cache:
       	Average read speed              : 41.3833 MB/s
      	Total bytes read in 1 minute    : 2.4 GB

2. Without cache:
	Average read speed		: 58.86 MB/s
	Total bytes read in 1 minute	: 3.3 GB

This PR also fixes a small issue with the boolean condition in table.Open().

(cherry picked from commit 91c31eb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants