-
Notifications
You must be signed in to change notification settings - Fork 289
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
perf(session): Avoid select when canceling session
When a resource relating to a session is deleted, notably a target, host, or auth_token, the corresponding sessions are marked as canceled via a database trigger that calls the cancel_session function. This function was performing a select as part of the insert call to ensure that it would not attempt to insert a session_sate of canceled if it was an invalid state transition. However, since this function was first introduced, there has been additional constraints added that would prevent an invalid state transition. In addition, performing a select as part of an insert increases the chances for contention with other queries that interact with the session_state table, especially since a large number of sessions and session_state rows can be included in a single transaction when things like auth_tokens or targets get deleted. Now, the cancel_function will perform the insert, and catch any of the potential violations, allowing the query to continue without needing to perform the select, thus reduction the potential for contention.
- Loading branch information
Showing
3 changed files
with
139 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
internal/db/schema/migrations/oss/postgres/91/07_cancel_session_trigger.up.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
-- Copyright (c) HashiCorp, Inc. | ||
-- SPDX-License-Identifier: BUSL-1.1 | ||
|
||
begin; | ||
|
||
-- Replaces function from 29/01_cancel_session_null_fkey.up.sql | ||
drop function cancel_session; | ||
create function cancel_session(sessionid text) returns void | ||
as $$ | ||
declare | ||
rows_affected numeric; | ||
begin | ||
insert into session_state(session_id, state) | ||
values (sessionId, 'canceling'); | ||
exception when unique_violation | ||
or foreign_key_violation | ||
or check_violation | ||
then | ||
-- Do nothing. Any one of these violations would indicate that the session | ||
-- either already is canceled, or is in a terminated state and cannot | ||
-- be canceled. | ||
end; | ||
$$ language plpgsql; | ||
commit; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
-- Copyright (c) HashiCorp, Inc. | ||
-- SPDX-License-Identifier: BUSL-1.1 | ||
|
||
begin; | ||
select plan(22); | ||
|
||
-- Canceling a terminated session should cause no errors, but should not change state. | ||
prepare cancel_terminated as | ||
select cancel_session('s1_____cindy'); | ||
-- to start s1_____cindy should have 2 states, and the terminated should be the active state. | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____cindy'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____cindy' | ||
and state = 'terminated' | ||
and upper(active_time_range) is null; | ||
-- now attempt to cancel | ||
select lives_ok('cancel_terminated'); | ||
-- there should be no changes to the state | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____cindy'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____cindy' | ||
and state = 'terminated' | ||
and upper(active_time_range) is null; | ||
|
||
-- Canceling a canceling session should cause no errors, but should not change state. | ||
prepare cancel_canceling as | ||
select cancel_session('s1_____ciara'); | ||
-- to start s1_____ciara should have 2 states, and the canceling should be the active state. | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____ciara'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____ciara' | ||
and state = 'canceling' | ||
and upper(active_time_range) is null; | ||
-- now attempt to cancel | ||
select lives_ok('cancel_canceling'); | ||
-- there should be no changes to the state | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____ciara'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____ciara' | ||
and state = 'canceling' | ||
and upper(active_time_range) is null; | ||
|
||
-- Canceling an active session should cause no errors and should result the state changing to canceling. | ||
prepare cancel_active as | ||
select cancel_session('s1_____carly'); | ||
-- to start s1_____carly should have 2 states, and the active should be the active state. | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____carly'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____carly' | ||
and state = 'active' | ||
and upper(active_time_range) is null; | ||
-- now attempt to cancel | ||
select lives_ok('cancel_active'); | ||
-- there should be a new state and active should no long be the active state. | ||
select is(count(*), 3::bigint) | ||
from session_state | ||
where session_id = 's1_____carly'; | ||
select is(count(*), 0::bigint) | ||
from session_state | ||
where session_id = 's1_____carly' | ||
and state = 'active' | ||
and upper(active_time_range) is null; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____carly' | ||
and state = 'canceling' | ||
and upper(active_time_range) is null; | ||
|
||
-- Canceling a pending session should cause no errors and should result the state changing to canceling. | ||
prepare cancel_pending as | ||
select cancel_session('s1_____clare'); | ||
-- to start s1_____clare should have 2 states, and the active should be the active state. | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____clare'; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____clare' | ||
and state = 'pending' | ||
and upper(active_time_range) is null; | ||
-- now attempt to cancel | ||
select lives_ok('cancel_pending'); | ||
-- there should be a new state and pending should no long be the active state. | ||
select is(count(*), 2::bigint) | ||
from session_state | ||
where session_id = 's1_____clare'; | ||
select is(count(*), 0::bigint) | ||
from session_state | ||
where session_id = 's1_____clare' | ||
and state = 'pending' | ||
and upper(active_time_range) is null; | ||
select is(count(*), 1::bigint) | ||
from session_state | ||
where session_id = 's1_____clare' | ||
and state = 'canceling' | ||
and upper(active_time_range) is null; | ||
|
||
select * from finish(); | ||
rollback; |