Skip to content

Commit f3e1288

Browse files
authored
fix: rely on OS temp directory (#486)
This is an issue to impose a `/tmp` as a default temporary directory, even thought this is the right one. On some case, you'd end up having a `/tmp` which is not writable (k8s: securityContext -> readOnlyRootFilesystem: true) and you'd prefer to have a way to override this.
1 parent 07938b5 commit f3e1288

File tree

3 files changed

+6
-10
lines changed

3 files changed

+6
-10
lines changed

cache/redis_cache.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ const statsTimeout = 500 * time.Millisecond
3434
// result from redis in a temporary files before sending it to the http response
3535
const minTTLForRedisStreamingReader = 15 * time.Second
3636

37-
// tmpDir temporary path to store ongoing queries results
38-
const tmpDir = "/tmp"
3937
const redisTmpFilePrefix = "chproxyRedisTmp"
4038

4139
func newRedisCache(client redis.UniversalClient, cfg config.Cache) *redisCache {
@@ -155,7 +153,7 @@ func (r *redisCache) readResultsAboveLimit(offset int, stringKey string, metadat
155153
// nb: it would be better to retry the flow if such a failure happened but this requires a huge refactoring of proxy.go
156154

157155
if ttl <= minTTLForRedisStreamingReader {
158-
fileStream, err := newFileWriterReader(tmpDir)
156+
fileStream, err := newFileWriterReader(os.TempDir())
159157
if err != nil {
160158
return nil, err
161159
}

cache/redis_cache_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func TestSmallTTLOnBigPayloadAreCacheWithFile(t *testing.T) {
236236

237237
//simulate a value almost expired
238238
redis.SetTTL(key.String(), 2*time.Second)
239-
nbFileCacheBeforeGet, err := countFilesWithPrefix(tmpDir, redisTmpFilePrefix)
239+
nbFileCacheBeforeGet, err := countFilesWithPrefix(os.TempDir(), redisTmpFilePrefix)
240240
if err != nil {
241241
t.Fatalf("could not read directory %s", err)
242242
}
@@ -245,7 +245,7 @@ func TestSmallTTLOnBigPayloadAreCacheWithFile(t *testing.T) {
245245
if err != nil {
246246
t.Fatalf("expected cached to have the value")
247247
}
248-
nbFileCacheAfterGet, err := countFilesWithPrefix(tmpDir, redisTmpFilePrefix)
248+
nbFileCacheAfterGet, err := countFilesWithPrefix(os.TempDir(), redisTmpFilePrefix)
249249
if err != nil {
250250
t.Fatalf("could not read directory %s", err)
251251
}
@@ -261,7 +261,7 @@ func TestSmallTTLOnBigPayloadAreCacheWithFile(t *testing.T) {
261261
t.Fatalf("got a value different than the expected one len(value)=%d vs len(expectedValue)=%d", len(string(cachedValue)), len(expectedValue))
262262
}
263263
cachedData.Data.Close()
264-
nbFileCacheAfterClose, err := countFilesWithPrefix(tmpDir, redisTmpFilePrefix)
264+
nbFileCacheAfterClose, err := countFilesWithPrefix(os.TempDir(), redisTmpFilePrefix)
265265
if err != nil {
266266
t.Fatalf("could not read directory %s", err)
267267
}

proxy.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/http"
1111
"net/http/httputil"
1212
"net/url"
13+
"os"
1314
"strconv"
1415
"strings"
1516
"sync"
@@ -22,9 +23,6 @@ import (
2223
"github.com/prometheus/client_golang/prometheus"
2324
)
2425

25-
// tmpDir temporary path to store ongoing queries results
26-
const tmpDir = "/tmp"
27-
2826
// failedTransactionPrefix prefix added to the failed reason for concurrent queries registry
2927
const failedTransactionPrefix = "[concurrent query failed]"
3028

@@ -417,7 +415,7 @@ func (rp *reverseProxy) serveFromCache(s *scope, srw *statResponseWriter, req *h
417415

418416
// The response wasn't found in the cache.
419417
// Request it from clickhouse.
420-
tmpFileRespWriter, err := cache.NewTmpFileResponseWriter(srw, tmpDir)
418+
tmpFileRespWriter, err := cache.NewTmpFileResponseWriter(srw, os.TempDir())
421419
if err != nil {
422420
err = fmt.Errorf("%s: %w; query: %q", s, err, q)
423421
respondWith(srw, err, http.StatusInternalServerError)

0 commit comments

Comments
 (0)