Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

PMM-11699 Reviewed and updated the advisor checks for PostgreSQL#82

Open
nastena1606 wants to merge 2 commits into
mainfrom
PMM-11699-PG-advisors
Open

PMM-11699 Reviewed and updated the advisor checks for PostgreSQL#82
nastena1606 wants to merge 2 commits into
mainfrom
PMM-11699-PG-advisors

Conversation

@nastena1606
Copy link
Copy Markdown
Contributor

No description provided.

@render
Copy link
Copy Markdown

render Bot commented Mar 16, 2023

@rnovikovP rnovikovP requested a review from elchinoo March 16, 2023 11:17

## Description
When this parameter is enabled (which it is by default), the PostgreSQL server tries to make sures that updates are physically written to disk, by issuing fsync() system calls or various equivalent methods mentioned in wal_sync_method.
By default, `fsync` is enabled in PostgreSQL so that the PostgreSQL server tries to make sure that updates are physically written to disk by issuing `fsync()` system calls or various equivalent methods mentioned in `wal_sync_method`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wouldn't be better to simplify with something like:

Fsync` is enabled by default in PostgreSQL ...

Turning off `fsync` can slightly improve performance with the risk of unrecoverable data corruption in case of a power failure or system crash.

Turning off fsync can slightly improve performance with the risk of unrecoverable data corruption in case of a power failure or system crash.
Therefore, it is only advisable to turn off `fsync` if you can easily recreate your entire database from external data.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would rephrase here to emphasize even more that it isn't advisable, for example:

It's not advisable to turn off fsync unless the database can be easily recreated from an external data source.

From the PostgreSQL log we should be able to get insight about the failure, we can even try the command manually to verify the result.

Something to keep in mind is the archive command will be executed under the ownership of the same user that the PostgreSQL server is running as, so the paths, tools, scripts, etc., that are called or acceded from the archive command should be accessible for the user running the service (usually postgres).
Something to keep in mind is that the same user that runs the PostgreSQL server (usually postgres), will execute the archive command, so the paths, tools, scripts, etc., that are called or acceded from the archive command should be accessible for the user running the service.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure if "acceded" would be the best wording here. I couldn't easily understand what it intends to say in this context. We may simplify a bit? We should remember that many no-native speakers will use this documentation, and using simpler daily vocabulary is a must.


## Description
When this parameter is enabled (which it is by default), the PostgreSQL server tries to make sures that updates are physically written to disk, by issuing fsync() system calls or various equivalent methods mentioned in wal_sync_method.
By default, `fsync` is enabled in PostgreSQL so that the PostgreSQL server tries to make sure that updates are physically written to disk by issuing `fsync()` system calls or various equivalent methods mentioned in `wal_sync_method`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
By default, `fsync` is enabled in PostgreSQL so that the PostgreSQL server tries to make sure that updates are physically written to disk by issuing `fsync()` system calls or various equivalent methods mentioned in `wal_sync_method`.
By default, `fsync` is enabled in PostgreSQL. This enables the PostgreSQL server to ensure that updates are physically written to disk. This is done by issuing `fsync()` system calls or various equivalent methods mentioned in the `wal_sync_method`.

When this parameter is enabled (which it is by default), the PostgreSQL server tries to make sures that updates are physically written to disk, by issuing fsync() system calls or various equivalent methods mentioned in wal_sync_method.
By default, `fsync` is enabled in PostgreSQL so that the PostgreSQL server tries to make sure that updates are physically written to disk by issuing `fsync()` system calls or various equivalent methods mentioned in `wal_sync_method`.

Turning off `fsync` can slightly improve performance with the risk of unrecoverable data corruption in case of a power failure or system crash.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Turning off `fsync` can slightly improve performance with the risk of unrecoverable data corruption in case of a power failure or system crash.
Turning off `fsync` can slightly improve performance. However, this comes with the risk of unrecoverable data corruption in the case of a power failure or system crash.

```
if members_number < 3:
```
This advisor returns a warning if the replica set has less than 3 members.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
This advisor returns a warning if the replica set has less than 3 members.
This advisor check warns if the replica set has less than three members.

@rnovikovP rnovikovP removed the request for review from Tusamarco April 3, 2023 08:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants