solver: fix resource limit merge semantics for CpuShares, MemorySwap, and CpuPeriod

Signed-off-by: Jiří Moravčík <jiri.moravcik@gmail.com>

Author: jirimoravcik

solver/jobs.go

@@ -1318,8 +1318,8 @@ func mergeLinuxResourcesRelaxed(a, b *pb.LinuxResources) *pb.LinuxResources {
 	}
 	return &pb.LinuxResources{
 		Memory:     relaxedLimit(a.Memory, b.Memory),
-		MemorySwap: relaxedLimit(a.MemorySwap, b.MemorySwap),
-		CpuShares:  relaxedLimit(a.CpuShares, b.CpuShares),
+		MemorySwap: relaxedMemorySwap(a.MemorySwap, b.MemorySwap),
+		CpuShares:  relaxedWeight(a.CpuShares, b.CpuShares),
 		CpuPeriod:  relaxedCPUPeriod(a.CpuPeriod, b.CpuPeriod, a.CpuQuota, b.CpuQuota),
 		CpuQuota:   relaxedLimit(a.CpuQuota, b.CpuQuota),
 		CpusetCpus: relaxedCpuset(a.CpusetCpus, b.CpusetCpus),
@@ -1336,12 +1336,51 @@ func relaxedLimit[T int64 | uint64](a, b T) T {
 	return max(a, b)
 }
 
+// relaxedMemorySwap returns the more relaxed of two memory-swap limits.
+// Per Docker semantics: 0 means "unset" (preserve the other value),
+// -1 means "unlimited swap" (most relaxed), and positive values are limits
+// where higher = more relaxed.
+func relaxedMemorySwap(a, b int64) int64 {
+	if a == 0 {
+		return b
+	}
+	if b == 0 {
+		return a
+	}
+	// -1 means unlimited swap, which is the most relaxed.
+	if a == -1 || b == -1 {
+		return -1
+	}
+	return max(a, b)
+}
+
+// relaxedWeight returns the more relaxed of two resource weights (like CPU shares).
+// Unlike limits, 0 means "unset" (use kernel default), not "unlimited".
+// Between two explicit values, higher weight = more resources = more relaxed.
+func relaxedWeight(a, b uint64) uint64 {
+	if a == 0 {
+		return b
+	}
+	if b == 0 {
+		return a
+	}
+	return max(a, b)
+}
+
 // relaxedCPUPeriod returns the CPU period corresponding to the more relaxed
 // CPU quota. Period is meaningless without quota, so we pick the period from
-// whichever side has the more relaxed quota. When both quotas are equal
-// (including both zero/unlimited), periodB is returned arbitrarily.
+// whichever side has the more relaxed quota. When both quotas are equal,
+// shorter period is more relaxed (same quota executes more frequently).
 func relaxedCPUPeriod(periodA, periodB uint64, quotaA, quotaB int64) uint64 {
 	relaxedQ := relaxedLimit(quotaA, quotaB)
+	if quotaA == quotaB {
+		// Equal quotas: shorter period = more CPU time = more relaxed.
+		if periodA == 0 || periodB == 0 {
+			// 0 means unset; preserve the set value.
+			return max(periodA, periodB)
+		}
+		return min(periodA, periodB)
+	}
 	if relaxedQ == quotaB {
 		return periodB
 	}

solver/merge_linux_resources_test.go

@@ -45,13 +45,53 @@ func TestMergeLinuxResourcesRelaxed(t *testing.T) {
 		require.Equal(t, int64(0), result.Memory)
 	})
 
+	t.Run("zero memory swap means unset, preserves non-zero", func(t *testing.T) {
+		a := &pb.LinuxResources{MemorySwap: 256 * 1024 * 1024}
+		b := &pb.LinuxResources{MemorySwap: 0}
+		result := mergeLinuxResourcesRelaxed(a, b)
+		require.Equal(t, int64(256*1024*1024), result.MemorySwap)
+
+		// Reverse order
+		result = mergeLinuxResourcesRelaxed(b, a)
+		require.Equal(t, int64(256*1024*1024), result.MemorySwap)
+	})
+
+	t.Run("memory swap -1 means unlimited and wins", func(t *testing.T) {
+		a := &pb.LinuxResources{MemorySwap: 256 * 1024 * 1024}
+		b := &pb.LinuxResources{MemorySwap: -1}
+		result := mergeLinuxResourcesRelaxed(a, b)
+		require.Equal(t, int64(-1), result.MemorySwap)
+
+		// Reverse order
+		result = mergeLinuxResourcesRelaxed(b, a)
+		require.Equal(t, int64(-1), result.MemorySwap)
+	})
+
+	t.Run("higher memory swap wins", func(t *testing.T) {
+		a := &pb.LinuxResources{MemorySwap: 128 * 1024 * 1024}
+		b := &pb.LinuxResources{MemorySwap: 256 * 1024 * 1024}
+		result := mergeLinuxResourcesRelaxed(a, b)
+		require.Equal(t, int64(256*1024*1024), result.MemorySwap)
+	})
+
 	t.Run("higher cpu shares wins", func(t *testing.T) {
 		a := &pb.LinuxResources{CpuShares: 256}
 		b := &pb.LinuxResources{CpuShares: 512}
 		result := mergeLinuxResourcesRelaxed(a, b)
 		require.Equal(t, uint64(512), result.CpuShares)
 	})
 
+	t.Run("zero cpu shares means unset, preserves non-zero", func(t *testing.T) {
+		a := &pb.LinuxResources{CpuShares: 512}
+		b := &pb.LinuxResources{CpuShares: 0}
+		result := mergeLinuxResourcesRelaxed(a, b)
+		require.Equal(t, uint64(512), result.CpuShares)
+
+		// Reverse order
+		result = mergeLinuxResourcesRelaxed(b, a)
+		require.Equal(t, uint64(512), result.CpuShares)
+	})
+
 	t.Run("higher cpu quota wins", func(t *testing.T) {
 		a := &pb.LinuxResources{CpuQuota: 50000, CpuPeriod: 100000}
 		b := &pb.LinuxResources{CpuQuota: 80000, CpuPeriod: 100000}
@@ -75,6 +115,14 @@ func TestMergeLinuxResourcesRelaxed(t *testing.T) {
 		require.Equal(t, uint64(200000), result.CpuPeriod)
 	})
 
+	t.Run("equal quotas picks shorter period (more relaxed)", func(t *testing.T) {
+		a := &pb.LinuxResources{CpuQuota: 50000, CpuPeriod: 100000}
+		b := &pb.LinuxResources{CpuQuota: 50000, CpuPeriod: 200000}
+		result := mergeLinuxResourcesRelaxed(a, b)
+		require.Equal(t, int64(50000), result.CpuQuota)
+		require.Equal(t, uint64(100000), result.CpuPeriod) // shorter period = more CPU
+	})
+
 	t.Run("empty cpuset string means unset and wins", func(t *testing.T) {
 		a := &pb.LinuxResources{CpusetCpus: "0-3"}
 		b := &pb.LinuxResources{CpusetCpus: ""}