Skip to content

Conversation

@eradman
Copy link
Contributor

@eradman eradman commented Jun 17, 2021

sysctlbyname(3) is a convenience wrapper for sysctl(3) that is implemented on FreeBSD and MacOS, but not on OpenBSD.

Include sys/param.h to ensure that BSD is defined.

Copy link
Collaborator

@DimCitus DimCitus left a comment

Choose a reason for hiding this comment

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

I think we can improve the style to look more like what we do elsewhere in the code. Also, can you list which OSes you have tested already with the changes?

@DimCitus DimCitus added enhancement New feature or request good first issue Good for newcomers Size: S Effort Estimate: Small labels Jun 17, 2021
sysctlbyname(3) is a convenience wrapper for sysctl(3) that is
implemented on FreeBSD and MacOS, but not on OpenBSD.

Include sys/param.h to ensure that BSD is defined.

Tests on OpenBSD:
- make check
- initialize a monitor node and 2 nodes
- perform failover

Tests on MacOS:
- make check
@DimCitus
Copy link
Collaborator

Hi @eradman ; I just had a try of your branch locally on my laptop, and it fails to detect the amount of RAM properly when using your current code. I suppose the old API needs to use another MIB?

- 12:23:02 43377 DEBUG pgtuning.c:85: Detected 12 CPUs and 16 GB total RAM on this server
+ 12:22:26 42248 DEBUG pgtuning.c:85: Detected 12 CPUs and 2048 MB total RAM on this server

That's the diff when running the following command once in the master's branch, then once in your PR branch:

$ PG_AUTOCTL_DEBUG=1 ./src/bin/pg_autoctl/pg_autoctl do pgsetup tune --pgdata /tmp -vv

Tested on

- MacOS 11.4
- FreeBSD 11.4
- OpenBSD 6.9

PG_AUTOCTL_DEBUG=1 pg_autoctl do pgsetup tune --pgdata /tmp -vv
Copy link
Contributor Author

@eradman eradman left a comment

Choose a reason for hiding this comment

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

This change was not nearly as easy as I thought since sysctl API has diverged between various BSD-like platforms

Other options:

  • Patch this in OpenBSD ports
  • Create a compatibility function sysctlbyname(3) for OpenBSD

@DimCitus
Copy link
Collaborator

I just tried it with a macOS and confirm this now works, and I think we're good with the code the way you've submitted it. I trust you have validated OpenBSD and FreeBSD variants, and intend to merge after Travis confirmed a test run.

@DimCitus DimCitus added this to the Sprint 2021 W26 W27 milestone Jun 28, 2021
@DimCitus DimCitus merged commit 2eb2197 into hapostgres:master Jun 28, 2021
@DimCitus
Copy link
Collaborator

Thanks for your contribution @eradman !

@eradman eradman deleted the get_system_info branch June 28, 2021 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request good first issue Good for newcomers Size: S Effort Estimate: Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants