Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(neonctl): set effective_io_concurrency to 0 on macos #10375

Closed
wants to merge 1 commit into from

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Jan 13, 2025

Problem

Postgres doesn't start with

2025-01-13 18:19:06.437 GMT [18213] DETAIL:  effective_io_concurrency must be set to 0 on platforms that lack posix_fadvise().

Summary of changes

Set the config item to 0 on macOS by default.

Signed-off-by: Alex Chi Z <chi@neon.tech>
@skyzh skyzh requested a review from jcsp January 13, 2025 18:21
@knizhnik
Copy link
Contributor

Sorry, we build Postgres at MacOS with -DUSE_PREFETCH:

else ifeq ($(UNAME_S),Darwin)
	PG_CFLAGS += -DUSE_PREFETCH

Setting this flag actually disable this check:

bool
check_effective_io_concurrency(int *newval, void **extra, GucSource source)
{
#ifndef USE_PREFETCH
	if (*newval != 0)
	{
		GUC_check_errdetail("\"%s\" must be set to 0 on platforms that lack support for issuing read-ahead advice.",
							"effective_io_concurrency");
		return false;
	}
#endif							/* USE_PREFETCH */
	return true;
}

So looks like for some reasons Darwin is not reported by name in your case.

@skyzh
Copy link
Member Author

skyzh commented Jan 13, 2025

Oh, seems like it's because I didn't reconfigure Postgres

@skyzh
Copy link
Member Author

skyzh commented Jan 13, 2025

Regardless I still prefer to get this patch merged because the fake cplane code should provide a correct config instead of relying on the Postgres code to automatically overwrite the value.

@knizhnik
Copy link
Contributor

Regardless I still prefer to get this patch merged because the fake cplane code should provide a correct config instead of relying on the Postgres code to automatically overwrite the value.

Sorry, I do not completely understand the concern.
But for testing purposes it is definitely better and more convenient to have prefetch enabled (and working) at MacOS.

I performing many tests and experiments with prefetch at MacOS and explicit changing of effective_io_concurrency in each case is very invonvenient.

So I really prefer to leave everything as it is.

Copy link

7304 tests run: 6934 passed, 0 failed, 370 skipped (full report)


Code coverage* (full report)

  • functions: 32.7% (8057 of 24666 functions)
  • lines: 47.8% (67021 of 140345 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
9e860e3 at 2025-01-13T20:16:33.052Z :recycle:

@skyzh skyzh closed this Jan 13, 2025
@bayandin bayandin deleted the skyzh/io-concurrency-macos branch January 14, 2025 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants