loading up the forgejo repo on tangled to test page performance
Fix content holes in Actions task logs file (#25560)
Fix #25451.
Bugfixes:
- When stopping the zombie or endless tasks, set `LogInStorage` to true
after transferring the file to storage. It was missing, it could write
to a nonexistent file in DBFS because `LogInStorage` was false.
- Always update `ActionTask.Updated` when there's a new state reported
by the runner, even if there's no change. This is to avoid the task
being judged as a zombie task.
Enhancement:
- Support `Stat()` for DBFS file.
- `WriteLogs` refuses to write if it could result in content holes.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
authored by
Jason Song
Giteabot
and committed by
GitHub
6daf21c9
b6693a2c
+98
-5
+9
models/actions/task.go
+9
models/actions/task.go
···
344
344
return err
345
345
}
346
346
347
+
// UpdateTaskByState updates the task by the state.
348
+
// It will always update the task if the state is not final, even there is no change.
349
+
// So it will update ActionTask.Updated to avoid the task being judged as a zombie task.
347
350
func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionTask, error) {
348
351
stepStates := map[int64]*runnerv1.StepState{}
349
352
for _, v := range state.Steps {
···
382
385
Status: task.Status,
383
386
Stopped: task.Stopped,
384
387
}, nil); err != nil {
388
+
return nil, err
389
+
}
390
+
} else {
391
+
// Force update ActionTask.Updated to avoid the task being judged as a zombie task
392
+
task.Updated = timeutil.TimeStampNow()
393
+
if err := UpdateTask(ctx, task, "updated"); err != nil {
385
394
return nil, err
386
395
}
387
396
}
+18
models/dbfs/dbfile.go
+18
models/dbfs/dbfile.go
···
7
7
"context"
8
8
"errors"
9
9
"io"
10
+
"io/fs"
10
11
"os"
11
12
"path/filepath"
12
13
"strconv"
···
21
22
type File interface {
22
23
io.ReadWriteCloser
23
24
io.Seeker
25
+
fs.File
24
26
}
25
27
26
28
type file struct {
···
193
195
return nil
194
196
}
195
197
198
+
func (f *file) Stat() (os.FileInfo, error) {
199
+
if f.metaID == 0 {
200
+
return nil, os.ErrInvalid
201
+
}
202
+
203
+
fileMeta, err := findFileMetaByID(f.ctx, f.metaID)
204
+
if err != nil {
205
+
return nil, err
206
+
}
207
+
return fileMeta, nil
208
+
}
209
+
196
210
func timeToFileTimestamp(t time.Time) int64 {
197
211
return t.UnixMicro()
212
+
}
213
+
214
+
func fileTimestampToTime(timestamp int64) time.Time {
215
+
return time.UnixMicro(timestamp)
198
216
}
199
217
200
218
func (f *file) loadMetaByPath() (*dbfsMeta, error) {
+29
models/dbfs/dbfs.go
+29
models/dbfs/dbfs.go
···
5
5
6
6
import (
7
7
"context"
8
+
"io/fs"
8
9
"os"
10
+
"path"
11
+
"time"
9
12
10
13
"code.gitea.io/gitea/models/db"
11
14
)
···
100
103
defer f.Close()
101
104
return f.delete()
102
105
}
106
+
107
+
var _ fs.FileInfo = (*dbfsMeta)(nil)
108
+
109
+
func (m *dbfsMeta) Name() string {
110
+
return path.Base(m.FullPath)
111
+
}
112
+
113
+
func (m *dbfsMeta) Size() int64 {
114
+
return m.FileSize
115
+
}
116
+
117
+
func (m *dbfsMeta) Mode() fs.FileMode {
118
+
return os.ModePerm
119
+
}
120
+
121
+
func (m *dbfsMeta) ModTime() time.Time {
122
+
return fileTimestampToTime(m.ModifyTimestamp)
123
+
}
124
+
125
+
func (m *dbfsMeta) IsDir() bool {
126
+
return false
127
+
}
128
+
129
+
func (m *dbfsMeta) Sys() any {
130
+
return nil
131
+
}
+13
models/dbfs/dbfs_test.go
+13
models/dbfs/dbfs_test.go
···
111
111
112
112
_, err = OpenFile(db.DefaultContext, "test2.txt", os.O_RDONLY)
113
113
assert.Error(t, err)
114
+
115
+
// test stat
116
+
f, err = OpenFile(db.DefaultContext, "test/test.txt", os.O_RDWR|os.O_CREATE)
117
+
assert.NoError(t, err)
118
+
stat, err := f.Stat()
119
+
assert.NoError(t, err)
120
+
assert.EqualValues(t, "test.txt", stat.Name())
121
+
assert.EqualValues(t, 0, stat.Size())
122
+
_, err = f.Write([]byte("0123456789"))
123
+
assert.NoError(t, err)
124
+
stat, err = f.Stat()
125
+
assert.NoError(t, err)
126
+
assert.EqualValues(t, 10, stat.Size())
114
127
}
115
128
116
129
func TestDbfsReadWrite(t *testing.T) {
+17
-1
modules/actions/log.go
+17
-1
modules/actions/log.go
···
29
29
)
30
30
31
31
func WriteLogs(ctx context.Context, filename string, offset int64, rows []*runnerv1.LogRow) ([]int, error) {
32
+
flag := os.O_WRONLY
33
+
if offset == 0 {
34
+
// Create file only if offset is 0, or it could result in content holes if the file doesn't exist.
35
+
flag |= os.O_CREATE
36
+
}
32
37
name := DBFSPrefix + filename
33
-
f, err := dbfs.OpenFile(ctx, name, os.O_WRONLY|os.O_CREATE)
38
+
f, err := dbfs.OpenFile(ctx, name, flag)
34
39
if err != nil {
35
40
return nil, fmt.Errorf("dbfs OpenFile %q: %w", name, err)
36
41
}
37
42
defer f.Close()
43
+
44
+
stat, err := f.Stat()
45
+
if err != nil {
46
+
return nil, fmt.Errorf("dbfs Stat %q: %w", name, err)
47
+
}
48
+
if stat.Size() < offset {
49
+
// If the size is less than offset, refuse to write, or it could result in content holes.
50
+
// However, if the size is greater than offset, we can still write to overwrite the content.
51
+
return nil, fmt.Errorf("size of %q is less than offset", name)
52
+
}
53
+
38
54
if _, err := f.Seek(offset, io.SeekStart); err != nil {
39
55
return nil, fmt.Errorf("dbfs Seek %q: %w", name, err)
40
56
}
+12
-4
services/actions/clear_tasks.go
+12
-4
services/actions/clear_tasks.go
···
56
56
return nil
57
57
}); err != nil {
58
58
log.Warn("Cannot stop task %v: %v", task.ID, err)
59
-
// go on
60
-
} else if remove, err := actions.TransferLogs(ctx, task.LogFilename); err != nil {
59
+
continue
60
+
}
61
+
62
+
remove, err := actions.TransferLogs(ctx, task.LogFilename)
63
+
if err != nil {
61
64
log.Warn("Cannot transfer logs of task %v: %v", task.ID, err)
62
-
} else {
63
-
remove()
65
+
continue
64
66
}
67
+
task.LogInStorage = true
68
+
if err := actions_model.UpdateTask(ctx, task, "log_in_storage"); err != nil {
69
+
log.Warn("Cannot update task %v: %v", task.ID, err)
70
+
continue
71
+
}
72
+
remove()
65
73
}
66
74
67
75
CreateCommitStatus(ctx, jobs...)