Skip to content

Avoid frequent checkpoints triggered by optimistic insertions#20336

Merged
Mytherin merged 1 commit intoduckdb:mainfrom
Captain32:fix_optimistic_insertion_trigger_checkpoint
Jan 7, 2026
Merged

Avoid frequent checkpoints triggered by optimistic insertions#20336
Mytherin merged 1 commit intoduckdb:mainfrom
Captain32:fix_optimistic_insertion_trigger_checkpoint

Conversation

@Captain32
Copy link
Contributor

See issue #20335 for details.

@Captain32 Captain32 force-pushed the fix_optimistic_insertion_trigger_checkpoint branch from 47e3341 to e985f28 Compare December 29, 2025 12:25
@duckdb-draftbot duckdb-draftbot marked this pull request as draft December 29, 2025 12:26
@Captain32 Captain32 marked this pull request as ready for review December 29, 2025 12:26
@Captain32 Captain32 force-pushed the fix_optimistic_insertion_trigger_checkpoint branch from e985f28 to f8af7bd Compare December 31, 2025 07:25
@duckdb-draftbot duckdb-draftbot marked this pull request as draft December 31, 2025 07:26
@Captain32 Captain32 marked this pull request as ready for review December 31, 2025 07:26
@Captain32
Copy link
Contributor Author

Let me explain my modifications to test/configs/peg_parser_strict.json.

{
  "description": "Test PEG Parser + transformer in strict mode",
  "skip_compiled": "true",
  "on_init": "set allow_parser_override_extension=strict_when_supported;",
  "statically_loaded_extensions": [
    "core_functions",
    "autocomplete"
  ],
  ...
}

As you can see, in this configuration, allow_parser_override_extension is set to strict_when_supported, and the loaded extensions include autocomplete.

  1. LOAD autocomplete will add autocomplete to parser_extensions:
static void LoadInternal(ExtensionLoader &loader) {
	TableFunction auto_complete_fun("sql_auto_complete", {LogicalType::VARCHAR}, SQLAutoCompleteFunction,
	                                SQLAutoCompleteBind, SQLAutoCompleteInit);
	...
	// LOAD `autocomplete` will add it to `parser_extensions`.
	auto &config = DBConfig::GetConfig(loader.GetDatabaseInstance());
	config.parser_extensions.push_back(PEGParserExtension());
}
  1. FORCE CHECKPOINT memory_compressed in in_memory_compress.test will be translated to CALL system.main.force_checkpoint('memory_compressed')
  2. The CALL command failed to be parsed due to the presence of autocomplete:
void Parser::ParseQuery(const string &query) {
	...
	string parser_override_option = StringUtil::Lower(options.parser_override_setting);
	...
	{
		if (options.extensions) {
			for (auto &ext : *options.extensions) {
				...
				if (StringUtil::CIEquals(parser_override_option, "strict_when_supported")) {
					...
					bool is_supported = false;
					switch (statement->type) {
					case StatementType::CALL_STATEMENT:
					...
						is_supported = true;
						break;
					...
					}
					if (is_supported) {
						// An extension exists, and parsing the CALL command failed!
						ThrowParserOverrideError(result);
					}
				}
			}
			...
		}
		...
	}
	...
}

Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I think this is fine. We used to rely on CHECKPOINT in order to persist optimistically written data - but since #13372 we can write the block pointers to the WAL as well. Estimating to zero does not seem right though. The size of the entries could be estimated to e.g. row_group_count * sizeof(PersistentColumnData) * types.size().

Connection con(db);

// Configuration to lower the checkpoint_threshold
REQUIRE_NO_FAIL(con.Query("PRAGMA disable_checkpoint_on_shutdown;"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be able to be rewritten to a SQL logic test - can we do that? We strongly prefer SQL logic tests over C++ tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment! I have rewritten the test case to a SQL logic test, see test/sql/storage/optimistic_write/optimistic_write_not_trigger_checkpoint.test for details.

By the way, I've added an estimate of the data block pointer size for optimistic insertion:

data_size = row_group_count * (sizeof(PersistentRowGroupData) +
		                       column_count * (sizeof(PersistentColumnData) + sizeof(DataPointer)));

@Captain32 Captain32 force-pushed the fix_optimistic_insertion_trigger_checkpoint branch from f8af7bd to fa97b62 Compare January 6, 2026 09:10
@duckdb-draftbot duckdb-draftbot marked this pull request as draft January 6, 2026 09:10
@Mytherin Mytherin marked this pull request as ready for review January 6, 2026 09:19
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

@Mytherin Mytherin merged commit 3f4d7b1 into duckdb:main Jan 7, 2026
102 checks passed
github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Feb 26, 2026
TIME_NS Support + Arrow (duckdb/duckdb#20361)
No catalog found Nightly test fix (duckdb/duckdb#20409)
Avoid frequent checkpoints triggered by optimistic insertions (duckdb/duckdb#20336)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants