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

we should enable data page checksums #87

Closed
jberkus opened this issue Oct 30, 2015 · 9 comments
Closed

we should enable data page checksums #87

jberkus opened this issue Oct 30, 2015 · 9 comments

Comments

@jberkus
Copy link
Contributor

jberkus commented Oct 30, 2015

If patroni is initdb'ing, we should enable data page checksums because:

  • it's a good idea
  • performance impact is not noticeable under most circumstances
  • prior to 9.5, it's required for pg_rewind to be safe to use
@CyberDem0n
Copy link
Member

I guess we should solve this problem more general. We can create special section in the config.yaml file which will contain all needed parameters and options for initdb

@jberkus
Copy link
Contributor Author

jberkus commented Nov 2, 2015

Why?

If Patroni is doing initdb, I don't see why it should give the user an option to turn off checksums.

@a1exsh
Copy link
Contributor

a1exsh commented Nov 3, 2015

Do we have any recent benchmark measuring performance penalty of enabling
data checksums? Last time I saw one, the effect was not nearly negligible,
I believe.

On Tue, Nov 3, 2015, 00:21 Josh Berkus notifications@github.com wrote:

Why?

If Patroni is doing initdb, I don't see why it should give the user an
option to turn off checksums.


Reply to this email directly or view it on GitHub
#87 (comment).

@jberkus
Copy link
Contributor Author

jberkus commented Nov 3, 2015

Hmmm, I need to publish. On workloads I've tested overhead has been < 5%. Compared to the safety of detecting corruption as soon as it happens, that's negligable. Also, I'm pretty sure that pg_rewind without checksums is unsafe, and you're building pg_rewind into Patroni.

@a1exsh
Copy link
Contributor

a1exsh commented Nov 3, 2015

Would be glad to see the benchmark. :-) pg_rewind requires checksums OR
logging of hint bits iirc.

On Tue, Nov 3, 2015, 19:38 Josh Berkus notifications@github.com wrote:

Hmmm, I need to publish. On workloads I've tested overhead has been < 5%.
Compared to the safety of detecting corruption as soon as it happens,
that's negligable. Also, I'm pretty sure that pg_rewind without checksums
is unsafe, and you're building pg_rewind into Patroni.


Reply to this email directly or view it on GitHub
#87 (comment).

@alexeyklyukin
Copy link
Contributor

Patroni can be attached to an already running cluster, or maybe in the future used to initialise new replicas out of the master that is completely unaware of patroni. On both of those cases we cannot guarantee the checksums will be on. We might solve this by fetching the checksum value parameter out of an existing master, but I think it would be easier to just make initdb options configurable.

@jberkus
Copy link
Contributor Author

jberkus commented Nov 10, 2015

We don't initdb in either of those cases.

@alexeyklyukin
Copy link
Contributor

Oh, right. But we also won't be able to rely on the checksums being on for pg_rewind in those cases. I think we should make this option configurable and choose the default value depending on the results of the benchmarks.

@feikesteenbergen
Copy link
Contributor

Implemented with custom options.

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

No branches or pull requests

5 participants