Skip to content

Commit

Permalink
hotfix: reset memoryUsageCache for each test, TestThresholdCrossing w…
Browse files Browse the repository at this point in the history
…orks with 64 bytes now due to a discovered pitfall on bytes.Buffer
  • Loading branch information
equals215 committed Jan 21, 2025
1 parent 2fb6a1a commit a6012f8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
2 changes: 2 additions & 0 deletions pkg/spooledtempfile/spooled.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ type ReadWriteSeekCloser interface {
//
// If the system memory usage is above maxRAMUsageFraction, we skip writing
// to memory and spool directly on disk to avoid OOM scenarios in high concurrency.
//
// From our tests, we've seen that a bytes.Buffer minimum allocated size is 64 bytes, any threshold below that will cause the first write to be spooled on disk.
func NewSpooledTempFile(
filePrefix string,
tempDir string,
Expand Down
22 changes: 20 additions & 2 deletions pkg/spooledtempfile/spooled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

// TestInMemoryBasic writes data below threshold and verifies it remains in memory.
func TestInMemoryBasic(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", os.TempDir(), 100, false, -1)
defer spool.Close()

Expand Down Expand Up @@ -62,11 +63,19 @@ func TestInMemoryBasic(t *testing.T) {

// TestThresholdCrossing writes enough data to switch from in-memory to disk.
func TestThresholdCrossing(t *testing.T) {
spool := NewSpooledTempFile("test", os.TempDir(), 16, false, -1)
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", os.TempDir(), 64, false, -1)
spoolFile := spool.(*spooledTempFile)
defer spool.Close()

data1 := []byte("12345")
data2 := []byte("67890ABCDEFGHIJKLM") // total length > 16
// Generate 64 random bytes in data2
data2 := make([]byte, 64)
_, _ = io.ReadFull(bytes.NewReader([]byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")), data2)

if spoolFile.buf.Cap() > 64 {
t.Errorf("Expected buffer capacity to be 64 or less, got %d", spoolFile.buf.Cap())
}

_, err := spool.Write(data1)
if err != nil {
Expand Down Expand Up @@ -105,6 +114,7 @@ func TestThresholdCrossing(t *testing.T) {

// TestForceOnDisk checks the fullOnDisk parameter.
func TestForceOnDisk(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", os.TempDir(), 1000000, true, -1)
defer spool.Close()

Expand All @@ -129,6 +139,7 @@ func TestForceOnDisk(t *testing.T) {

// TestReadAtAndSeekInMemory tests seeking and ReadAt on an in-memory spool.
func TestReadAtAndSeekInMemory(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 100, false, -1)
defer spool.Close()

Expand Down Expand Up @@ -173,6 +184,7 @@ func TestReadAtAndSeekInMemory(t *testing.T) {

// TestReadAtAndSeekOnDisk tests seeking and ReadAt on a spool that has switched to disk.
func TestReadAtAndSeekOnDisk(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 10, false, -1)
defer spool.Close()

Expand Down Expand Up @@ -204,6 +216,7 @@ func TestReadAtAndSeekOnDisk(t *testing.T) {

// TestWriteAfterReadPanic ensures writing after reading panics per your design.
func TestWriteAfterReadPanic(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 100, false, -1)
defer spool.Close()

Expand Down Expand Up @@ -237,6 +250,7 @@ func TestWriteAfterReadPanic(t *testing.T) {

// TestCloseInMemory checks closing while still in-memory.
func TestCloseInMemory(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 100, false, -1)

_, err := spool.Write([]byte("Small data"))
Expand Down Expand Up @@ -267,6 +281,7 @@ func TestCloseInMemory(t *testing.T) {

// TestCloseOnDisk checks closing after spool has switched to disk.
func TestCloseOnDisk(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 10, false, -1)

_, err := spool.Write([]byte("1234567890ABC"))
Expand Down Expand Up @@ -305,6 +320,7 @@ func TestCloseOnDisk(t *testing.T) {

// TestLen verifies Len() for both in-memory and on-disk states.
func TestLen(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("test", "", 5, false, -1)
defer spool.Close()

Expand All @@ -329,6 +345,7 @@ func TestLen(t *testing.T) {

// TestFileName checks correctness of FileName in both modes.
func TestFileName(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
spool := NewSpooledTempFile("testprefix", os.TempDir(), 5, false, -1)
defer spool.Close()

Expand Down Expand Up @@ -357,6 +374,7 @@ func TestFileName(t *testing.T) {
// TestSkipInMemoryAboveRAMUsage verifies that if `isSystemMemoryUsageHigh()`
// returns true, the spool goes directly to disk even for small writes.
func TestSkipInMemoryAboveRAMUsage(t *testing.T) {
memoryUsageCache = &globalMemoryCache{}
// Save the old function so we can restore it later
oldGetSystemMemoryUsedFraction := getSystemMemoryUsedFraction
// Force system memory usage to appear above 50%
Expand Down

0 comments on commit a6012f8

Please sign in to comment.