Skip to content

Commit 9c411d3

Browse files
committed
Make response file name prefix configurable
Stale files can get large and need cleaning up, then it helps to have a descriptive name (rather than just 'tmp')
1 parent 77a99f1 commit 9c411d3

9 files changed

Lines changed: 83 additions & 12 deletions

File tree

cache/async_cache.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ type AsyncCache struct {
2222

2323
MaxPayloadSize config.ByteSize
2424
SharedWithAllUsers bool
25+
TmpFilePrefix string
2526
}
2627

2728
func (c *AsyncCache) Close() error {
@@ -114,5 +115,6 @@ func NewAsyncCache(cfg config.Cache, maxExecutionTime time.Duration) (*AsyncCach
114115
graceTime: graceTime,
115116
MaxPayloadSize: maxPayloadSize,
116117
SharedWithAllUsers: cfg.SharedWithAllUsers,
118+
TmpFilePrefix: cfg.TmpFilePrefix,
117119
}, nil
118120
}

cache/filesystem_cache_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func TestCacheClean(t *testing.T) {
194194
Query: []byte(fmt.Sprintf("SELECT %d cache clean", i)),
195195
}
196196
trw := &testResponseWriter{}
197-
crw, err := NewTmpFileResponseWriter(trw, testTmpWriterDir)
197+
crw, err := NewTmpFileResponseWriter(trw, testTmpWriterDir, DefaultTmpFilePrefix)
198198
if err != nil {
199199
t.Fatalf("create tmp cache: %s", err)
200200
}

cache/tmp_file_response_writer.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,18 @@ type TmpFileResponseWriter struct {
2525
bw *bufio.Writer // buffered writer for the temporary file
2626
}
2727

28-
func NewTmpFileResponseWriter(rw http.ResponseWriter, dir string) (*TmpFileResponseWriter, error) {
28+
const DefaultTmpFilePrefix = "chproxyTmp"
29+
30+
func NewTmpFileResponseWriter(rw http.ResponseWriter, dir, prefix string) (*TmpFileResponseWriter, error) {
2931
_, ok := rw.(http.CloseNotifier)
3032
if !ok {
3133
return nil, fmt.Errorf("the response writer does not implement http.CloseNotifier")
3234
}
35+
if prefix == "" {
36+
prefix = DefaultTmpFilePrefix
37+
}
3338

34-
f, err := os.CreateTemp(dir, "tmp")
39+
f, err := os.CreateTemp(dir, prefix)
3540
if err != nil {
3641
return nil, fmt.Errorf("cannot create temporary file in %q: %w", dir, err)
3742
}

cache/tmp_file_response_writer_test.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"log"
66
"net/http"
77
"os"
8+
"path/filepath"
9+
"strings"
810
"testing"
911
)
1012

@@ -60,7 +62,7 @@ func TestFileCreation(t *testing.T) {
6062
files, _ := os.ReadDir(testTmpWriterDir)
6163
nbFileBefore := len(files)
6264

63-
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir)
65+
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, DefaultTmpFilePrefix)
6466
defer tmpFileRespWriter.Close()
6567
if err != nil {
6668
t.Fatalf("could not initate TmpFileResponseWriter error:%s", err)
@@ -80,7 +82,7 @@ func TestFileRemoval(t *testing.T) {
8082
files, _ := os.ReadDir(testTmpWriterDir)
8183
nbFileBefore := len(files)
8284

83-
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir)
85+
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, DefaultTmpFilePrefix)
8486
if err != nil {
8587
t.Fatalf("could not initate TmpFileResponseWriter error:%s", err)
8688
return
@@ -98,7 +100,7 @@ func TestFileRemoval(t *testing.T) {
98100
func TestWriteThenReadHeader(t *testing.T) {
99101
srw := newFakeResponse()
100102

101-
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir)
103+
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, DefaultTmpFilePrefix)
102104
defer tmpFileRespWriter.Close()
103105
if err != nil {
104106
t.Fatalf("could not initate TmpFileResponseWriter error:%s", err)
@@ -129,7 +131,7 @@ func TestWriteThenReadHeader(t *testing.T) {
129131
func TestWriteThenReadContent(t *testing.T) {
130132
srw := newFakeResponse()
131133

132-
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir)
134+
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, DefaultTmpFilePrefix)
133135
defer tmpFileRespWriter.Close()
134136
if err != nil {
135137
t.Fatalf("could not initate TmpFileResponseWriter error:%s", err)
@@ -167,7 +169,7 @@ func TestWriteThenReadStatusCode(t *testing.T) {
167169
srw := newFakeResponse()
168170
expectStatusCode1 := http.StatusOK
169171
expectStatusCode2 := 444
170-
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir)
172+
tmpFileRespWriter, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, DefaultTmpFilePrefix)
171173
defer tmpFileRespWriter.Close()
172174
if err != nil {
173175
t.Fatalf("could not initate TmpFileResponseWriter error:%s", err)
@@ -188,3 +190,19 @@ func TestWriteThenReadStatusCode(t *testing.T) {
188190
}
189191

190192
}
193+
194+
func TestTmpFilePrefixIsUsed(t *testing.T) {
195+
const customPrefix = "myCustomPrefix"
196+
srw := newFakeResponse()
197+
198+
w, err := NewTmpFileResponseWriter(srw, testTmpWriterDir, customPrefix)
199+
if err != nil {
200+
t.Fatalf("could not create TmpFileResponseWriter: %s", err)
201+
}
202+
defer w.Close()
203+
204+
name := filepath.Base(w.tmpFile.Name())
205+
if !strings.HasPrefix(name, customPrefix) {
206+
t.Fatalf("expected tmp file name to start with %q, got %q", customPrefix, name)
207+
}
208+
}

config/config.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -936,14 +936,18 @@ type Cache struct {
936936

937937
Redis RedisCacheConfig `yaml:"redis,omitempty"`
938938

939-
// Catches all undefined fields
940-
XXX map[string]interface{} `yaml:",inline"`
941-
942939
// Maximum total size of request payload for caching
943940
MaxPayloadSize ByteSize `yaml:"max_payload_size,omitempty"`
944941

945942
// Whether a query cached by a user could be used by another user
946943
SharedWithAllUsers bool `yaml:"shared_with_all_users,omitempty"`
944+
945+
// Prefix for temporary files created during response caching.
946+
// Defaults to "chproxyTmp" if not set.
947+
TmpFilePrefix string `yaml:"tmp_file_prefix,omitempty"`
948+
949+
// Catches all undefined fields
950+
XXX map[string]interface{} `yaml:",inline"`
947951
}
948952

949953
func (c *Cache) setDefaults() {

config/config_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ var fullConfig = Config{
3131
GraceTime: Duration(20 * time.Second),
3232
MaxPayloadSize: ByteSize(100 << 30),
3333
SharedWithAllUsers: false,
34+
TmpFilePrefix: "chproxyTmp",
3435
},
3536
{
3637
Name: "shortterm",
@@ -983,3 +984,21 @@ func TestConfigReplaceEnvVars(t *testing.T) {
983984
})
984985
}
985986
}
987+
988+
func TestCacheTmpFilePrefixParsed(t *testing.T) {
989+
const input = `
990+
name: testcache
991+
mode: file_system
992+
tmp_file_prefix: chproxyResponseCache_
993+
file_system:
994+
dir: /tmp
995+
max_size: 100Mb
996+
`
997+
var c Cache
998+
if err := yaml.Unmarshal([]byte(input), &c); err != nil {
999+
t.Fatalf("unexpected parse error: %s", err)
1000+
}
1001+
if c.TmpFilePrefix != "chproxyResponseCache_" {
1002+
t.Fatalf("expected TmpFilePrefix %q, got %q", "chproxyResponseCache_", c.TmpFilePrefix)
1003+
}
1004+
}

config/testdata/full.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ caches:
3333

3434
max_payload_size: 100Gb
3535

36+
# Optional prefix for temporary files created while streaming a response
37+
# into the cache. Useful for distinguishing chproxy temp files from other
38+
# processes when inspecting the OS temp directory.
39+
#
40+
# Defaults to "chproxyTmp".
41+
tmp_file_prefix: chproxyTmp
42+
3643
# Expiration time for cached responses.
3744
expire: 1h
3845

docs/src/content/docs/configuration/caching.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,22 @@ User Y will get the cached response from user X's query.
6363
Since 1.20.0, the cache is specific for each user by default since it's better in terms of security.
6464
It's possible to use the previous behavior by setting the following property of the cache in the config file `shared_with_all_users = true`
6565

66+
#### Temporary file prefix
67+
68+
While streaming a ClickHouse response into the cache, chproxy writes it to a temporary file in the OS temp directory. The filename starts with a configurable prefix, which defaults to `chproxyTmp`.
69+
70+
Setting a custom prefix is useful when you need to distinguish chproxy temp files from those of other processes, for example in monitoring or cleanup scripts:
71+
72+
```yaml
73+
caches:
74+
- name: my-cache
75+
mode: file_system
76+
tmp_file_prefix: chproxyResponseCache_
77+
file_system:
78+
dir: /path/to/cache
79+
max_size: 100Mb
80+
```
81+
6682
#### Detecting Cache Hits
6783
6884
`Chproxy` will respond with an `X-Cache` header with a value of `HIT` if it returned a response from either the local or the distributed cache. Otherwise `X-Cache` will be set to `MISS`.

proxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ func (rp *reverseProxy) serveFromCache(s *scope, srw *statResponseWriter, req *h
418418

419419
// The response wasn't found in the cache.
420420
// Request it from clickhouse.
421-
tmpFileRespWriter, err := cache.NewTmpFileResponseWriter(srw, os.TempDir())
421+
tmpFileRespWriter, err := cache.NewTmpFileResponseWriter(srw, os.TempDir(), userCache.TmpFilePrefix)
422422
if err != nil {
423423
err = fmt.Errorf("%s: %w; query: %q", s, err, q)
424424
respondWith(srw, err, http.StatusInternalServerError)

0 commit comments

Comments
 (0)