objstore: set correct part size in KS3 multipart uploads

Project: pingcap/tidb | Language: Go | Concepts: testing-strategy, configuration-management | Difficulty: mid
Link: https://github.com/pingcap/tidb/pull/70696

The problem

The KS3 object storage backend for TiDB was not honoring the configured multipart upload part size. Although the configuration allowed specifying a part size, the code always set a fixed 5 MiB size internally, causing the underlying SDK to ignore the user's configuration. This could lead to multipart uploads using incorrect part sizes, potentially resulting in upload failures or inefficiencies for large files.

Why it's non-trivial

A naive fix might simply change the value of the part size in the configuration or in a single place, but the real problem was that the code explicitly set the UploadInput.Size field to a fixed value. This hardcoded value overrode any configuration, making it necessary to remove or correctly set this field so the SDK would use the intended part size. Additionally, testing this required simulating multipart uploads and verifying the actual part sizes sent to the backend, not just the configuration values.

The approach

1. Remove the hardcoded part size from the upload input

Previously, the code always set the upload part size to 5 MiB:

-			Size:   1024 * 1024 * 5, // ks3 SDK need to set this value to non-zero.

This line was removed:

-			Size:   1024 * 1024 * 5, // ks3 SDK need to set this value to non-zero.

Annotation:
By removing this line, the SDK now uses the PartSize specified in the upload options, allowing the user’s configuration to take effect.

2. Add a regression test to verify correct part sizes are used

A new test function was added to ensure that the multipart upload honors the configured part size. The test sets up a fake HTTP server to simulate the KS3 API and tracks the sizes of uploaded parts:

func TestKS3CreateHonorsPartSize(t *testing.T) {
	const (
		partSize = 6 * 1024 * 1024
		uploadID = "test-upload-id"
	)

	var (
		mu                sync.Mutex
		uploadedPartSizes = make(map[string]int)
	)
	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		// ... handle multipart upload requests and record part sizes ...
	}))
	defer server.Close()

	ctx := context.Background()
	storage, err := NewKS3Storage(ctx, &backuppb.S3{
		// ... config ...
	}, &storeapi.Options{})
	require.NoError(t, err)

	writer, err := storage.Create(ctx, "file", &storeapi.WriterOption{
		Concurrency: 2,
		PartSize:    partSize,
	})
	require.NoError(t, err)

	data := make([]byte, 2*partSize+1024)
	n, err := writer.Write(ctx, data)
	require.NoError(t, err)
	require.Equal(t, len(data), n)
	require.NoError(t, writer.Close(ctx))

	mu.Lock()
	defer mu.Unlock()
	require.Equal(t, map[string]int{
		"1": partSize,
		"2": partSize,
		"3": 1024,
	}, uploadedPartSizes)
}

Annotation:
This test simulates a multipart upload with a part size of 6 MiB, then uploads data large enough to require three parts. It checks that the actual HTTP requests sent to the server match the expected part sizes, confirming the configuration is respected.

From the review thread

No notable review discussion on this PR.

Takeaways

• Avoid hardcoding configuration values in code paths that are meant to be user-configurable; always defer to the configuration where possible.
• When fixing configuration-related bugs, add regression tests that verify the system’s observable behavior (e.g., actual network requests), not just internal state or configuration fields.
• For storage integrations, simulate the backend in tests to verify protocol-level correctness, especially for multipart or chunked operations.