Make concurrent database writes wait instead of failing (closes #253)

SQLite transactions started deferred, so two that both read and then
wrote could not both proceed, and SQLite refused the second at once with
"database is locked" without waiting. TestCreateUserRaceCondition hit
this now and then.

The database is now opened with _txlock=immediate, so each transaction
takes the write lock when it begins and a second one waits for it, and
with _busy_timeout=5000, so that wait lasts up to 5 seconds. The driver
already defaults to 5000; it is now stated in the code.

Model: opus-5-5

README.md

@@ -278,6 +278,9 @@ exist yet, upaas has Docker create it as an empty directory, owned by root, when
 the app's container starts; there is no need to create it first. An existing
 host path is left as it is. This needs Docker Engine 23.0 or later.
 
+A deploy or rollback replaces the app's container with a new one, which keeps
+only the files the app wrote to its volume mounts.
+
 ## License
 
 WTFPL

TODO.md

@@ -24,6 +24,16 @@ regress.
   "database is locked": each transaction now takes the write lock when it begins
   and waits up to 5 seconds for another writer to finish (#253).
 
+- 2026-10-01: The hint under the app page's environment variable editor now says
+  changes take effect at the next deploy or rollback, in the page's warning
+  style, instead of asking for a container restart, which keeps the old values
+  (#255).
+
+- 2026-10-01: An app with no volume mounts says on its page, and in the log of
+  each deploy, that a deploy or rollback loses the files it writes and a restart
+  keeps them; the README's Volume mounts section says a deploy or rollback keeps
+  only the files the app wrote to its volume mounts (#248).
+
 - 2026-10-01: Builds attach a BuildKit session, as the docker command line does,
   so a base image that is not on the host is pulled instead of the build failing
   with "no active sessions" on Docker Engine 27. Container logs, and so the

internal/handlers/app.go

@@ -21,6 +21,7 @@ import (
 	"sneak.berlin/go/upaas/internal/database"
 	"sneak.berlin/go/upaas/internal/models"
 	"sneak.berlin/go/upaas/internal/service/app"
+	"sneak.berlin/go/upaas/internal/service/deploy"
 	"sneak.berlin/go/upaas/templates"
 )
 
@@ -194,6 +195,7 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc {
 			"EnvVars":          envVars,
 			"Labels":           labels,
 			"Volumes":          volumes,
+			"NoVolumesWarning": deploy.NoVolumesWarning,
 			"Ports":            ports,
 			"Deployments":      deployments,
 			"LatestDeployment": latestDeployment,

internal/handlers/app_no_volumes_test.go

@@ -0,0 +1,45 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/upaas/internal/models"
+	"sneak.berlin/go/upaas/internal/service/deploy"
+)
+
+// TestAppPageSaysFilesAreLostOnlyWhenAppHasNoVolumes checks that the app page
+// shows deploy.NoVolumesWarning until the app gets a volume mount.
+func TestAppPageSaysFilesAreLostOnlyWhenAppHasNoVolumes(t *testing.T) {
+	t.Parallel()
+
+	testCtx := setupTestHandlers(t)
+	createdApp := createTestApp(t, testCtx, "no-volumes-app")
+
+	renderAppPage := func() string {
+		request := httptest.NewRequestWithContext(
+			t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil,
+		)
+		request = addChiURLParams(request, map[string]string{"id": createdApp.ID})
+		recorder := httptest.NewRecorder()
+
+		testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request)
+		require.Equal(t, http.StatusOK, recorder.Code)
+
+		return recorder.Body.String()
+	}
+
+	assert.Contains(t, renderAppPage(), deploy.NoVolumesWarning)
+
+	volume := models.NewVolume(testCtx.database)
+	volume.AppID = createdApp.ID
+	volume.HostPath = "/srv/no-volumes-app"
+	volume.ContainerPath = "/data"
+	require.NoError(t, volume.Save(t.Context()))
+
+	assert.NotContains(t, renderAppPage(), deploy.NoVolumesWarning)
+}

internal/handlers/app_page_env_var_hint_test.go

@@ -0,0 +1,39 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/upaas/internal/service/app"
+)
+
+// TestAppPageEnvVarHint checks that the environment variable editor says
+// changes take effect at the next deploy or rollback, in the warning style.
+func TestAppPageEnvVarHint(t *testing.T) {
+	t.Parallel()
+
+	testCtx := setupTestHandlers(t)
+
+	createdApp, err := testCtx.appSvc.CreateApp(t.Context(), app.CreateAppInput{
+		Name:    "env-hint-app",
+		RepoURL: "git@example.com:user/env-hint-app.git",
+	})
+	require.NoError(t, err)
+
+	request := httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil,
+	)
+	request = addChiURLParams(request, map[string]string{"id": createdApp.ID})
+	recorder := httptest.NewRecorder()
+
+	testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request)
+
+	require.Equal(t, http.StatusOK, recorder.Code)
+	assert.Contains(t, recorder.Body.String(),
+		`<p class="alert-warning mt-1">`+
+			"Environment variable changes take effect at the next deploy or rollback.</p>")
+}

internal/service/deploy/deploy.go

@@ -56,6 +56,12 @@ var (
 	ErrNoPreviousImage = errors.New("no previous image available for rollback")
 )
 
+// NoVolumesWarning is shown on the page of an app with no volume mounts and
+// written into each of its deploy logs.
+const NoVolumesWarning = "This app has no volume mounts, so the files it writes " +
+	"are lost whenever a deploy or rollback replaces its container; " +
+	"a restart of the container keeps them."
+
 // logFlushInterval is how often to flush buffered logs to the database.
 const logFlushInterval = time.Second
 
@@ -369,6 +375,11 @@ func (svc *Service) Deploy(
 
 	svc.logWebhookPayload(bgCtx, deployment, webhookEvent)
 
+	volumes, err := app.GetVolumes(bgCtx)
+	if err == nil && len(volumes) == 0 {
+		_ = deployment.AppendLog(bgCtx, NoVolumesWarning)
+	}
+
 	err = svc.updateAppStatusBuilding(bgCtx, app)
 	if err != nil {
 		return err

internal/service/deploy/deploy_volumes_test.go

@@ -0,0 +1,79 @@
+package deploy_test
+
+import (
+	"context"
+	"log/slog"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/upaas/internal/config"
+	"sneak.berlin/go/upaas/internal/database"
+	"sneak.berlin/go/upaas/internal/docker"
+	"sneak.berlin/go/upaas/internal/logger"
+	"sneak.berlin/go/upaas/internal/models"
+	"sneak.berlin/go/upaas/internal/service/deploy"
+	"sneak.berlin/go/upaas/internal/service/notify"
+)
+
+// deployLog deploys a new app, with one volume mount when withVolume is set,
+// and returns the deploy's log. Docker is not connected, so the deploy fails
+// at the git clone, after the start of its log is written.
+func deployLog(t *testing.T, withVolume bool) string {
+	t.Helper()
+
+	log := logger.NewForTest(slog.New(slog.NewTextHandler(os.Stderr, nil)))
+	dataDir := t.TempDir()
+	cfg := &config.Config{DataDir: dataDir, HostDataDir: dataDir}
+	db := database.NewTestDatabase(t)
+
+	dockerClient, err := docker.New(nil, docker.Params{Logger: log, Config: cfg})
+	require.NoError(t, err)
+
+	notifySvc, err := notify.New(nil, notify.ServiceParams{Logger: log})
+	require.NoError(t, err)
+
+	svc, err := deploy.New(nil, deploy.ServiceParams{
+		Logger: log, Config: cfg, Database: db,
+		Docker: dockerClient, Notify: notifySvc,
+	})
+	require.NoError(t, err)
+
+	ctx := context.Background()
+
+	app := models.NewApp(db)
+	app.ID = "volumesapp-id"
+	app.Name = "volumesapp"
+	app.Branch = "main"
+	require.NoError(t, app.Save(ctx))
+
+	if withVolume {
+		volume := models.NewVolume(db)
+		volume.AppID = app.ID
+		volume.HostPath = "/srv/volumesapp"
+		volume.ContainerPath = "/data"
+		require.NoError(t, volume.Save(ctx))
+	}
+
+	err = svc.Deploy(ctx, app, nil, false)
+	require.ErrorIs(t, err, docker.ErrNotConnected)
+
+	deployments, err := app.GetDeployments(ctx, 1)
+	require.NoError(t, err)
+	require.Len(t, deployments, 1)
+
+	return deployments[0].Logs.String
+}
+
+func TestDeployLogSaysFilesAreLostOnlyWhenAppHasNoVolumes(t *testing.T) {
+	t.Parallel()
+
+	logWithoutVolumes := deployLog(t, false)
+	assert.Equal(t, 1, strings.Count(logWithoutVolumes, deploy.NoVolumesWarning),
+		logWithoutVolumes)
+
+	assert.NotContains(t, deployLog(t, true), deploy.NoVolumesWarning)
+}

templates/app_detail.html

@@ -178,7 +178,7 @@
                                     <button type="submit" class="btn-primary text-sm">Save</button>
                                     <button type="button" @click="editIdx = -1" class="text-gray-500 hover:text-gray-700 text-sm">Cancel</button>
                                 </form>
-                                <p class="text-xs text-amber-600 mt-1">⚠ Container restart needed after env var changes.</p>
+                                <p class="alert-warning mt-1">Environment variable changes take effect at the next deploy or rollback.</p>
                             </td>
                         </template>
                     </tr>
@@ -318,6 +318,8 @@
                 </tbody>
             </table>
         </div>
+        {{else}}
+        <p class="alert-warning">{{.NoVolumesWarning}}</p>
         {{end}}
         <form method="POST" action="/apps/{{.App.ID}}/volumes" class="flex flex-col sm:flex-row gap-2 items-end">
                 {{ .CSRFField }}