fix(tasks): add labels sequentially when the backend db serializes writes
Quick Add Magic with multiple labels (`*a *b *c`) fired all
`PUT /tasks/{id}/labels` requests concurrently via `Promise.all`. On
SQLite these overlap as read-then-write upgrade transactions, which the
busy_timeout can't resolve, so some requests fail with HTTP 500
("database is locked") and the labels are silently dropped while the
quick-add input gets stuck.
Expose a `concurrent_writes` flag on the shared `/info` response (true
for Postgres/MySQL, false for SQLite). The frontend config store reads
it and `addLabelsToTask` now branches: parallel `Promise.all` when the
backend supports concurrent writes, sequential awaits otherwise.
Fixes #2680
This commit is contained in:
parent
f3c6312a9e
commit
6e851e2ec2
|
|
@ -47,6 +47,7 @@ export interface ConfigState {
|
|||
publicTeamsEnabled: boolean,
|
||||
allowIconChanges: boolean,
|
||||
enabledProFeatures: string[],
|
||||
concurrentWrites: boolean,
|
||||
}
|
||||
|
||||
export const useConfigStore = defineStore('config', () => {
|
||||
|
|
@ -88,6 +89,7 @@ export const useConfigStore = defineStore('config', () => {
|
|||
publicTeamsEnabled: false,
|
||||
allowIconChanges: true,
|
||||
enabledProFeatures: [],
|
||||
concurrentWrites: false,
|
||||
})
|
||||
|
||||
const migratorsEnabled = computed(() => state.availableMigrators?.length > 0)
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import {describe, expect, it} from 'vitest'
|
||||
import {buildDefaultRemindersForQuickAdd} from './tasks'
|
||||
import {buildDefaultRemindersForQuickAdd, runWrites} from './tasks'
|
||||
import {REMINDER_PERIOD_RELATIVE_TO_TYPES} from '@/types/IReminderPeriodRelativeTo'
|
||||
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
|
||||
|
||||
|
|
@ -42,3 +42,39 @@ describe('buildDefaultRemindersForQuickAdd', () => {
|
|||
expect(result[0].relativeTo).toBe(REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE)
|
||||
})
|
||||
})
|
||||
|
||||
describe('runWrites', () => {
|
||||
function deferredWrite() {
|
||||
const inFlight: string[] = []
|
||||
let maxConcurrent = 0
|
||||
const completed: string[] = []
|
||||
const write = async (item: string) => {
|
||||
inFlight.push(item)
|
||||
maxConcurrent = Math.max(maxConcurrent, inFlight.length)
|
||||
await Promise.resolve()
|
||||
inFlight.splice(inFlight.indexOf(item), 1)
|
||||
completed.push(item)
|
||||
}
|
||||
return {write, completed, getMaxConcurrent: () => maxConcurrent}
|
||||
}
|
||||
|
||||
it('runs all writes in parallel when concurrent', async () => {
|
||||
const {write, completed, getMaxConcurrent} = deferredWrite()
|
||||
await runWrites(['a', 'b', 'c'], write, true)
|
||||
expect(completed).toHaveLength(3)
|
||||
expect(getMaxConcurrent()).toBeGreaterThan(1)
|
||||
})
|
||||
|
||||
it('runs writes one at a time when not concurrent', async () => {
|
||||
const {write, completed, getMaxConcurrent} = deferredWrite()
|
||||
await runWrites(['a', 'b', 'c'], write, false)
|
||||
expect(completed).toEqual(['a', 'b', 'c'])
|
||||
expect(getMaxConcurrent()).toBe(1)
|
||||
})
|
||||
|
||||
it('does nothing for an empty list', async () => {
|
||||
const {write, completed} = deferredWrite()
|
||||
await runWrites([], write, false)
|
||||
expect(completed).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ import type {IProject} from '@/modelTypes/IProject'
|
|||
import {REMINDER_PERIOD_RELATIVE_TO_TYPES} from '@/types/IReminderPeriodRelativeTo'
|
||||
|
||||
import {setModuleLoading} from '@/stores/helper'
|
||||
import {useConfigStore} from '@/stores/config'
|
||||
import {useLabelStore} from '@/stores/labels'
|
||||
import {useProjectStore} from '@/stores/projects'
|
||||
import {useKanbanStore} from '@/stores/kanban'
|
||||
|
|
@ -59,6 +60,22 @@ export function buildDefaultRemindersForQuickAdd(
|
|||
}))
|
||||
}
|
||||
|
||||
// runWrites applies a write to each item. SQLite deadlocks on concurrent writes
|
||||
// (read-then-write upgrade conflict), so callers pass concurrent=false to serialize.
|
||||
export async function runWrites<T>(
|
||||
items: readonly T[],
|
||||
write: (item: T) => Promise<unknown>,
|
||||
concurrent: boolean,
|
||||
): Promise<void> {
|
||||
if (concurrent) {
|
||||
await Promise.all(items.map(item => write(item)))
|
||||
return
|
||||
}
|
||||
for (const item of items) {
|
||||
await write(item)
|
||||
}
|
||||
}
|
||||
|
||||
// IDEA: maybe use a small fuzzy search here to prevent errors
|
||||
function findPropertyByValue(object, key, value, fuzzy = false) {
|
||||
return Object.values(object).find(l => {
|
||||
|
|
@ -131,6 +148,7 @@ export const useTaskStore = defineStore('task', () => {
|
|||
const labelStore = useLabelStore()
|
||||
const projectStore = useProjectStore()
|
||||
const authStore = useAuthStore()
|
||||
const configStore = useConfigStore()
|
||||
|
||||
const tasks = ref<{ [id: ITask['id']]: ITask }>({}) // TODO: or is this ITask[]
|
||||
const isLoading = ref(false)
|
||||
|
|
@ -395,10 +413,7 @@ export const useTaskStore = defineStore('task', () => {
|
|||
}
|
||||
|
||||
const labels = await ensureLabelsExist(parsedLabels)
|
||||
const labelAddsToWaitFor = labels.map(async l => addLabelToTask(task, l))
|
||||
|
||||
// This waits until all labels are created and added to the task
|
||||
await Promise.all(labelAddsToWaitFor)
|
||||
await runWrites(labels, l => addLabelToTask(task, l), configStore.concurrentWrites)
|
||||
return task
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -54,6 +54,8 @@ type VikunjaInfos struct {
|
|||
PublicTeamsEnabled bool `json:"public_teams_enabled" doc:"Whether public teams are enabled."`
|
||||
AllowIconChanges bool `json:"allow_icon_changes" doc:"Whether users may change project icons."`
|
||||
EnabledProFeatures []license.Feature `json:"enabled_pro_features" doc:"The licensed pro features enabled on this instance."`
|
||||
// ConcurrentWrites reports whether the configured database can handle concurrent writes. It is false on SQLite, where overlapping write transactions deadlock, so clients should serialize batched writes instead of firing them in parallel.
|
||||
ConcurrentWrites bool `json:"concurrent_writes" doc:"Whether the configured database supports concurrent writes. False on SQLite; clients should serialize batched writes when this is false."`
|
||||
}
|
||||
|
||||
// AuthInfo describes the authentication methods enabled on this instance.
|
||||
|
|
@ -106,6 +108,7 @@ func BuildInfo() VikunjaInfos {
|
|||
WebhooksEnabled: config.WebhooksEnabled.GetBool(),
|
||||
PublicTeamsEnabled: config.ServiceEnablePublicTeams.GetBool(),
|
||||
AllowIconChanges: config.ServiceAllowIconChanges.GetBool(),
|
||||
ConcurrentWrites: config.DatabaseType.GetString() != "sqlite",
|
||||
EnabledProFeatures: license.EnabledProFeatures(),
|
||||
AvailableMigrators: []string{
|
||||
(&vikunja_file.FileMigrator{}).Name(),
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ import (
|
|||
"net/http"
|
||||
"testing"
|
||||
|
||||
"code.vikunja.io/api/pkg/config"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
|
@ -39,4 +41,7 @@ func TestHumaInfo(t *testing.T) {
|
|||
assert.Contains(t, body, "version")
|
||||
assert.Contains(t, body, "auth")
|
||||
assert.Contains(t, body, "available_migrators")
|
||||
|
||||
require.Contains(t, body, "concurrent_writes")
|
||||
assert.Equal(t, config.DatabaseType.GetString() != "sqlite", body["concurrent_writes"])
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue