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

"SELECT FOR UPDATE" is needed for PostgreSQL #3039

Closed
yangmarcyang opened this issue May 6, 2022 · 4 comments · Fixed by #3103
Closed

"SELECT FOR UPDATE" is needed for PostgreSQL #3039

yangmarcyang opened this issue May 6, 2022 · 4 comments · Fixed by #3103
Labels
priority/backlog Issue is approved and in the backlog
Milestone

Comments

@yangmarcyang
Copy link
Contributor

yangmarcyang commented May 6, 2022

Hi All,

In #2770, we encountered "extreme cases" where a JWT key exists in local journal.pem but does not exist in DB bundle table. Recently the issue happened again in our setup where a spire-server generated a new key, stored in DB bundle table without any error, saved into local journal.pem file, but unfortunately that key did not exist in DB bundle table.

After we set up testing environment and could reproduce the issue consistently, we found that the root cause is related with PostgreSQL hot standby mode. In our setup, hot standby mode is turned on, and according to the spec, "those using hot standby may want to use Repeatable Read and explicit locking on the master". So, "SELECT FOR UPDATE" is needed in this case.

We just simply copied this line out of the if block and ran it in the testing environment, the issue became non-reproducible.

So, to have consistency guarantees for PostgreSQL with hot standby mode, IMHO "SELECT FOR UPDATE" is still needed.

@amartinezfayo amartinezfayo added the triage/in-progress Issue triage is in progress label May 6, 2022
@amartinezfayo
Copy link
Member

@azdagron This behavior was introduced in #2605 to fix #2587. As @yangmarcyang points, data consistency checks have special considerations in PostgreSQL when hot standby mode is turned on. May we able to detect when this mode is turned on and have SELECT .. FOR UPDATE there also?

@loveyana
Copy link
Contributor

loveyana commented May 6, 2022

@azdagron This behavior was introduced in #2605 to fix #2587. As @yangmarcyang points, data consistency checks have special considerations in PostgreSQL when hot standby mode is turned on. May we able to detect when this mode is turned on and have SELECT .. FOR UPDATE there also?

@amartinezfayo I think we can tell if psql is in hot standby mode by show hot_standby is on and show wal_level is not minimal, but I'm not sure if this problem occurs in other psql configurations. If SELECT ... FOR UPDATE is currently fully enabled under mysql and it has less impact on performance, can we accept it as the default query option.

@azdagron
Copy link
Member

azdagron commented May 6, 2022

I don't see a problem just enabling SELECT FOR UPDATE on all read-modify-write tx's for both MySQL and PostgreSQL.

@amartinezfayo amartinezfayo added priority/backlog Issue is approved and in the backlog and removed triage/in-progress Issue triage is in progress labels May 6, 2022
@amartinezfayo amartinezfayo added this to the 1.3.1 milestone May 6, 2022
@amartinezfayo
Copy link
Member

Thanks @azdagron. I also think that there shouldn't be any problem enabling SELECT ... FOR UPDATE on all read-modify-write transactions for both MySQL and PostgreSQL.
I've added this to the 1.3.1 milestone.

azdagron added a commit to azdagron/spire that referenced this issue May 24, 2022
This is required to successfully perform read-modify-write operations
when PostgreSQL is in hot standby mode.

Fixes: spiffe#3039

Signed-off-by: Andrew Harding <aharding@vmware.com>
azdagron added a commit that referenced this issue May 24, 2022
This is required to successfully perform read-modify-write operations
when PostgreSQL is in hot standby mode.

Fixes: #3039

Signed-off-by: Andrew Harding <aharding@vmware.com>
stevend-uber pushed a commit to stevend-uber/spire that referenced this issue Oct 16, 2023
This is required to successfully perform read-modify-write operations
when PostgreSQL is in hot standby mode.

Fixes: spiffe#3039

Signed-off-by: Andrew Harding <aharding@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/backlog Issue is approved and in the backlog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants