Skip to content

Commit 8eddf1d

Browse files
committed
storagenode/hashstore: windows fixes
operators reported errors going from 1.141.4 to 1.142.4 on windows and the errors indicated that it was during atomic file rename. the two changes during that time were the switch from os.Rename to platform.Rename and the removal of share permissions on CreateFile. this change puts back the share permissions for the CreateFile api on windows. dunno why i removed it in the first place. it does keep the os.Rename to platform.Rename change because it should be the same except for the windows.MOVEFILE_WRITE_THROUGH which should make it more safe for atomic rename because it's documented to make it not return until the file is actually moved on disk. the tests pass on wine which is the best i can do right now. Change-Id: Ibe80fb078a928890d2badff5f99314777cabd8cc
1 parent d00fe29 commit 8eddf1d

File tree

2 files changed

+13
-64
lines changed

2 files changed

+13
-64
lines changed

storagenode/hashstore/crash_test.go

Lines changed: 12 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,17 @@ import (
2525
)
2626

2727
func TestMain(m *testing.M) {
28-
if os.Getenv("STORJ_HASHSTORE_CRASH_TEST") != "" {
29-
if err := runCrashServer(context.Background()); err != nil {
28+
if dir := os.Getenv("STORJ_HASHSTORE_CRASH_TEST"); dir != "" {
29+
if err := runCrashServer(context.Background(), dir); err != nil {
3030
log.Fatalf("%+v", err)
3131
}
3232
os.Exit(0)
3333
}
34-
3534
os.Exit(m.Run())
3635
}
3736

38-
func runCrashServer(ctx context.Context) error {
39-
tmpDir, err := os.MkdirTemp("", "hashstore-crash-test-*")
40-
if err != nil {
41-
return errs.Wrap(err)
42-
}
43-
fmt.Println(tmpDir)
44-
45-
db, err := New(ctx, CreateDefaultConfig(TableKind_HashTbl, false), tmpDir, "", nil, nil, nil)
37+
func runCrashServer(ctx context.Context, dir string) error {
38+
db, err := New(ctx, CreateDefaultConfig(TableKind_HashTbl, false), dir, "", nil, nil, nil)
4639
if err != nil {
4740
return errs.Wrap(err)
4841
}
@@ -102,9 +95,10 @@ func runCrashServer(ctx context.Context) error {
10295
func TestCorrectDuringCrashes(t *testing.T) {
10396
ctx := t.Context()
10497

105-
// re-exec the process with the STORJ_HASHSTORE_CRASH_TEST env var set
98+
// re-exec the process with the STORJ_HASHSTORE_CRASH_TEST env var set to a temp dir.
99+
dir := t.TempDir()
106100
cmd := exec.Command(os.Args[0], os.Args[1:]...)
107-
cmd.Env = append(os.Environ(), "STORJ_HASHSTORE_CRASH_TEST=server")
101+
cmd.Env = []string{"STORJ_HASHSTORE_CRASH_TEST=" + dir}
108102

109103
// give a stdin handle to the process so that it can know if the parent process dies.
110104
cmdStdinR, cmdStdinW, err := os.Pipe()
@@ -113,45 +107,23 @@ func TestCorrectDuringCrashes(t *testing.T) {
113107
defer func() { _ = cmdStdinW.Close() }()
114108
cmd.Stdin = cmdStdinR
115109

116-
// give a stdout handle so we can read the keys it prints out.
117-
var stdout lockedBuffer
110+
// capture stdout and stderr so we can read the keys it prints out and any errors.
111+
var stdout, stderr bytes.Buffer
118112
cmd.Stdout = &stdout
119-
120-
// give a stderr handle so we can look at any errors it prints out.
121-
var stderr bytes.Buffer
122113
cmd.Stderr = &stderr
123114

124115
// start the command and create a signal to keep track of when it finishes.
125116
assert.NoError(t, cmd.Start())
126117
var finished drpcsignal.Signal
127118
go func() { finished.Set(cmd.Wait()) }()
128119

129-
// wait until the directory is printed.
130-
var dir string
131-
for {
132-
if prefix, _, ok := strings.Cut(stdout.String(), "\n"); ok {
133-
dir = prefix
134-
break
135-
}
136-
137-
select {
138-
case <-finished.Signal():
139-
t.Fatal("process exited early:", finished.Err())
140-
case <-time.After(time.Millisecond):
141-
}
142-
}
143-
144-
// ensure the directory is what we expect and clean it up when we're done.
145-
assert.That(t, strings.HasPrefix(dir, os.TempDir()))
146-
defer func() { _ = os.RemoveAll(dir) }()
147-
148120
// wait until we have at least 3 hashtbl files (middle of a compaction).
149121
for {
150122
paths := allFiles(t, dir)
151123

152124
hashtbls := 0
153125
for _, path := range paths {
154-
if strings.Contains(path, "meta/hashtbl-") {
126+
if strings.Contains(path, "hashtbl-") {
155127
hashtbls++
156128
}
157129
}
@@ -172,12 +144,8 @@ func TestCorrectDuringCrashes(t *testing.T) {
172144

173145
// grab all of the keys out of stdout.
174146
var keys []Key
175-
for _, line := range strings.Split(stdout.String(), "\n")[1:] {
176-
if strings.TrimSpace(line) == "" {
177-
continue
178-
}
179-
180-
key, err := storj.PieceIDFromString(line)
147+
for line := range bytes.Lines(stdout.Bytes()) {
148+
key, err := storj.PieceIDFromString(string(line))
181149
assert.NoError(t, err)
182150
keys = append(keys, key)
183151
}
@@ -197,22 +165,3 @@ func TestCorrectDuringCrashes(t *testing.T) {
197165
assert.Equal(t, dataFromKey(key), got)
198166
}
199167
}
200-
201-
type lockedBuffer struct {
202-
buf bytes.Buffer
203-
mu sync.Mutex
204-
}
205-
206-
func (a *lockedBuffer) String() string {
207-
a.mu.Lock()
208-
defer a.mu.Unlock()
209-
210-
return a.buf.String()
211-
}
212-
213-
func (a *lockedBuffer) Write(p []byte) (n int, err error) {
214-
a.mu.Lock()
215-
defer a.mu.Unlock()
216-
217-
return a.buf.Write(p)
218-
}

storagenode/hashstore/platform/file_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func CreateFile(path string) (*os.File, error) {
3131
handle, err := windows.CreateFile(
3232
pathPtr,
3333
windows.GENERIC_READ|windows.GENERIC_WRITE,
34-
0,
34+
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
3535
nil,
3636
windows.CREATE_NEW,
3737
windows.FILE_ATTRIBUTE_NORMAL,

0 commit comments

Comments
 (0)