Skip to content

Commit 934a977

Browse files
ChrisPennerChrisPenner
authored
Merge pull request #69 from unisoncomputing/cp/freeze-contribution-causals
Store source/target causals on contribution itself, so they stay frozen once merged
2 parents c0a76b9 + 79481c8 commit 934a977

25 files changed

Lines changed: 264 additions & 103 deletions

‎share-api.cabal‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ library
2828
Share.App
2929
Share.Backend
3030
Share.BackgroundJobs
31-
Share.BackgroundJobs.Diffs.ContributionDiffs
31+
Share.BackgroundJobs.Diffs.CausalDiffs
3232
Share.BackgroundJobs.Diffs.Queries
33+
Share.BackgroundJobs.Diffs.Types
3334
Share.BackgroundJobs.Errors
3435
Share.BackgroundJobs.Monad
3536
Share.BackgroundJobs.Search.DefinitionSync

‎sql/2025-05-09_track_contribution_causals.sql‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
-- Track the causals on the contribution itself; this is useful because the branches may change _after_ the
2+
-- contribution is merged or closed.
3+
ALTER TABLE contributions
4+
ADD COLUMN source_causal_id INTEGER NULL REFERENCES causals (id) ON DELETE NO ACTION,
5+
ADD COLUMN target_causal_id INTEGER NULL REFERENCES causals (id) ON DELETE NO ACTION
6+
;
7+
8+
-- Backfill the new columns with current causal of each contribution, this will be incorrect for old contributions
9+
-- where we can't infer what the source branch _was_ at when it was merged.
10+
UPDATE contributions
11+
SET source_causal_id = (
12+
SELECT pb.causal_id
13+
FROM project_branches pb
14+
WHERE pb.id = contributions.source_branch
15+
LIMIT 1
16+
),
17+
target_causal_id = (
18+
SELECT pb.causal_id
19+
FROM project_branches pb
20+
WHERE pb.id = contributions.target_branch
21+
LIMIT 1
22+
)
23+
WHERE source_causal_id IS NULL
24+
AND target_causal_id IS NULL
25+
;
26+
27+
-- Make the new columns non-nullable
28+
ALTER TABLE contributions
29+
ALTER COLUMN source_causal_id SET NOT NULL,
30+
ALTER COLUMN target_causal_id SET NOT NULL
31+
;

‎sql/2025-05-12_causal_diff_queue.sql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
DROP TABLE contribution_diff_queue;
2+
3+
-- Table for causal diffs we want to compute.
4+
-- Also keyed by the codebase owner of each side of the diff since
5+
-- the sandboxed terms may affect how the diff looks.
6+
CREATE TABLE causal_diff_queue (
7+
from_causal_id INTEGER NOT NULL REFERENCES causals(id) ON DELETE CASCADE,
8+
to_causal_id INTEGER NOT NULL REFERENCES causals(id) ON DELETE CASCADE,
9+
from_codebase_owner UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
10+
to_codebase_owner UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
11+
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
12+
13+
PRIMARY KEY (from_causal_id, to_causal_id, from_codebase_owner, to_codebase_owner)
14+
);

‎src/Share/BackgroundJobs.hs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
module Share.BackgroundJobs (startWorkers) where
22

33
import Ki.Unlifted qualified as Ki
4-
import Share.BackgroundJobs.Diffs.ContributionDiffs qualified as ContributionDiffs
4+
import Share.BackgroundJobs.Diffs.CausalDiffs qualified as ContributionDiffs
55
import Share.BackgroundJobs.Monad (Background)
66
import Share.BackgroundJobs.Search.DefinitionSync qualified as DefnSearch
77
import Share.BackgroundJobs.Webhooks.Worker qualified as Webhooks

src/Share/BackgroundJobs/Diffs/ContributionDiffs.hs renamed to src/Share/BackgroundJobs/Diffs/CausalDiffs.hs

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,24 @@
1-
module Share.BackgroundJobs.Diffs.ContributionDiffs (worker) where
1+
module Share.BackgroundJobs.Diffs.CausalDiffs (worker) where
22

33
import Control.Lens
4-
import Control.Monad.Except
54
import Ki.Unlifted qualified as Ki
65
import Share.BackgroundJobs.Diffs.Queries qualified as DQ
6+
import Share.BackgroundJobs.Diffs.Types (CausalDiffInfo (..))
77
import Share.BackgroundJobs.Errors (reportError)
8-
import Share.BackgroundJobs.Monad (Background, withTag)
8+
import Share.BackgroundJobs.Monad (Background, withTags)
99
import Share.BackgroundJobs.Workers (newWorker)
10-
import Share.Branch (branchCausals_)
1110
import Share.Codebase qualified as Codebase
12-
import Share.Contribution (Contribution (..))
1311
import Share.Env qualified as Env
14-
import Share.IDs
1512
import Share.IDs qualified as IDs
1613
import Share.Metrics qualified as Metrics
1714
import Share.Postgres qualified as PG
15+
import Share.Postgres.Causal.Queries qualified as CausalQ
1816
import Share.Postgres.Contributions.Queries qualified as ContributionsQ
1917
import Share.Postgres.Notifications qualified as Notif
20-
import Share.Postgres.Queries qualified as Q
2118
import Share.Prelude
2219
import Share.Utils.Logging qualified as Logging
2320
import Share.Web.Authorization qualified as AuthZ
24-
import Share.Web.Errors (EntityMissing (..), ErrorID (..))
21+
import Share.Web.Errors (EntityMissing (..))
2522
import Share.Web.Share.Diffs.Impl qualified as Diffs
2623
import System.Clock qualified as Clock
2724

@@ -39,8 +36,8 @@ worker scope = do
3936
makeRuntime codebase = do
4037
runtime <- Codebase.codebaseRuntimeTransaction unisonRuntime codebase
4138
pure (badUnliftCodebaseRuntime runtime)
42-
newWorker scope "diffs:contributions" $ forever do
43-
Notif.waitOnChannel Notif.ContributionDiffChannel (maxPollingIntervalSeconds * 1000000)
39+
newWorker scope "causal-diffs" $ forever do
40+
Notif.waitOnChannel Notif.CausalDiffChannel (maxPollingIntervalSeconds * 1000000)
4441
processDiffs authZReceipt makeRuntime
4542

4643
-- Process diffs until we run out of them. We claim a diff in a transaction and compute the diff in the same
@@ -52,20 +49,26 @@ processDiffs authZReceipt makeRuntime = do
5249
loop = do
5350
result <-
5451
PG.runTransactionMode PG.RepeatableRead PG.ReadWrite do
55-
DQ.claimContributionToDiff >>= \case
52+
DQ.claimCausalDiff >>= \case
5653
Nothing -> pure Nothing
57-
Just contributionId -> do
54+
Just causalDiffInfo -> do
5855
startTime <- PG.transactionUnsafeIO (Clock.getTime Clock.Monotonic)
59-
result <- PG.catchTransaction (maybeComputeAndStoreCausalDiff authZReceipt makeRuntime contributionId)
60-
pure (Just (contributionId, startTime, result))
61-
whenJust result \(contributionId, startTime, result) -> do
62-
withTag "contribution-id" (IDs.toText contributionId) do
56+
result <- PG.catchTransaction (maybeComputeAndStoreCausalDiff authZReceipt makeRuntime causalDiffInfo)
57+
pure (Just (causalDiffInfo, startTime, result))
58+
whenJust result \(CausalDiffInfo {fromCausalId, toCausalId, fromCodebaseOwner, toCodebaseOwner}, startTime, result) -> do
59+
let tags =
60+
[ ("from-causal-id", IDs.toText fromCausalId),
61+
("to-causal-id", IDs.toText toCausalId),
62+
("from-codebase-owner", IDs.toText fromCodebaseOwner),
63+
("to-codebase-owner", IDs.toText toCodebaseOwner)
64+
]
65+
withTags tags do
6366
case result of
6467
Left err -> reportError err
6568
Right didWork -> do
6669
when didWork do
67-
liftIO (Metrics.recordContributionDiffDuration startTime)
68-
Logging.textLog "Computed contribution diff"
70+
liftIO (Metrics.recordCausalDiffDuration startTime)
71+
Logging.textLog "Computed causal diff"
6972
& Logging.withSeverity Logging.Info
7073
& Logging.logMsg
7174
loop
@@ -76,27 +79,21 @@ processDiffs authZReceipt makeRuntime = do
7679
maybeComputeAndStoreCausalDiff ::
7780
AuthZ.AuthZReceipt ->
7881
(Codebase.CodebaseEnv -> IO (Codebase.CodebaseRuntime IO)) ->
79-
ContributionId ->
82+
CausalDiffInfo ->
8083
PG.Transaction EntityMissing Bool
81-
maybeComputeAndStoreCausalDiff authZReceipt makeRuntime contributionId = do
82-
Contribution {bestCommonAncestorCausalId, sourceBranchId = newBranchId, targetBranchId = oldBranchId, projectId} <-
83-
ContributionsQ.contributionById contributionId
84-
project <- Q.projectById projectId `whenNothingM` throwError (EntityMissing (ErrorID "project:missing") "Project not found")
85-
newBranch <- Q.branchById newBranchId `whenNothingM` throwError (EntityMissing (ErrorID "branch:missing") "Source branch not found")
86-
oldBranch <- Q.branchById oldBranchId `whenNothingM` throwError (EntityMissing (ErrorID "branch:missing") "Target branch not found")
87-
let oldCodebase = Codebase.codebaseForProjectBranch authZReceipt project oldBranch
88-
let newCodebase = Codebase.codebaseForProjectBranch authZReceipt project newBranch
89-
let oldCausal = oldBranch ^. branchCausals_
90-
let newCausal = newBranch ^. branchCausals_
91-
ContributionsQ.existsPrecomputedNamespaceDiff (oldCodebase, oldCausal) (newCodebase, newCausal) >>= \case
84+
maybeComputeAndStoreCausalDiff authZReceipt makeRuntime (CausalDiffInfo {fromCausalId, toCausalId, fromCodebaseOwner, toCodebaseOwner}) = do
85+
bestCommonAncestorCausalId <- CausalQ.bestCommonAncestor fromCausalId toCausalId
86+
let fromCodebase = Codebase.codebaseEnv authZReceipt $ Codebase.codebaseLocationForUserCodebase fromCodebaseOwner
87+
let toCodebase = Codebase.codebaseEnv authZReceipt $ Codebase.codebaseLocationForUserCodebase toCodebaseOwner
88+
ContributionsQ.existsPrecomputedNamespaceDiff (fromCodebase, fromCausalId) (toCodebase, toCausalId) >>= \case
9289
True -> pure False
9390
False -> do
94-
oldRuntime <- PG.transactionUnsafeIO (makeRuntime oldCodebase)
95-
newRuntime <- PG.transactionUnsafeIO (makeRuntime newCodebase)
91+
fromRuntime <- PG.transactionUnsafeIO (makeRuntime fromCodebase)
92+
toRuntime <- PG.transactionUnsafeIO (makeRuntime toCodebase)
9693
_ <-
9794
Diffs.computeAndStoreCausalDiff
9895
authZReceipt
99-
(oldCodebase, oldRuntime, oldCausal)
100-
(newCodebase, newRuntime, newCausal)
96+
(fromCodebase, fromRuntime, fromCausalId)
97+
(toCodebase, toRuntime, toCausalId)
10198
bestCommonAncestorCausalId
10299
pure True

‎src/Share/BackgroundJobs/Diffs/Queries.hs‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
module Share.BackgroundJobs.Diffs.Queries
22
( submitContributionsToBeDiffed,
3-
claimContributionToDiff,
3+
claimCausalDiff,
44
)
55
where
66

7+
import Share.BackgroundJobs.Diffs.Types
78
import Share.IDs
89
import Share.Postgres
910
import Share.Postgres.Notifications qualified as Notif
@@ -15,28 +16,47 @@ submitContributionsToBeDiffed contributions = do
1516
[sql|
1617
WITH new_contributions(contribution_id) AS (
1718
SELECT * FROM ^{singleColumnTable (toList contributions)}
19+
), diff_info(from_causal_id, to_causal_id, from_codebase_owner, to_codebase_owner) AS (
20+
SELECT c.target_causal_id, c.source_causal_id, COALESCE(target_branch.contributor_id, target_project.owner_user_id), COALESCE(source_branch.contributor_id, source_project.owner_user_id)
21+
FROM new_contributions nc
22+
JOIN contributions c ON c.id = nc.contribution_id
23+
JOIN project_branches source_branch ON source_branch.id = c.source_branch
24+
JOIN project_branches target_branch ON target_branch.id = c.target_branch
25+
JOIN projects source_project ON source_project.id = source_branch.project_id
26+
JOIN projects target_project ON target_project.id = target_branch.project_id
1827
)
19-
INSERT INTO contribution_diff_queue (contribution_id)
20-
SELECT nc.contribution_id FROM new_contributions nc
28+
INSERT INTO causal_diff_queue (from_causal_id, to_causal_id, from_codebase_owner, to_codebase_owner)
29+
SELECT from_causal_id, to_causal_id, from_codebase_owner, to_codebase_owner
30+
FROM diff_info
31+
WHERE NOT EXISTS (
32+
SELECT FROM namespace_diffs nd
33+
WHERE nd.left_causal_id = diff_info.from_causal_id
34+
AND nd.right_causal_id = diff_info.to_causal_id
35+
AND nd.left_codebase_owner_user_id = diff_info.from_codebase_owner
36+
AND nd.right_codebase_owner_user_id = diff_info.to_codebase_owner
37+
)
2138
ON CONFLICT DO NOTHING
2239
|]
23-
Notif.notifyChannel Notif.ContributionDiffChannel
40+
Notif.notifyChannel Notif.CausalDiffChannel
2441

2542
-- | Claim the oldest contribution in the queue to be diffed.
26-
claimContributionToDiff :: Transaction e (Maybe ContributionId)
27-
claimContributionToDiff = do
28-
query1Col
43+
claimCausalDiff :: Transaction e (Maybe CausalDiffInfo)
44+
claimCausalDiff = do
45+
query1Row
2946
[sql|
30-
WITH chosen_contribution(contribution_id) AS (
31-
SELECT q.contribution_id
32-
FROM contribution_diff_queue q
47+
WITH chosen(from_causal_id, to_causal_id, from_codebase_owner, to_codebase_owner) AS (
48+
SELECT q.from_causal_id, q.to_causal_id, q.from_codebase_owner, q.to_codebase_owner
49+
FROM causal_diff_queue q
3350
ORDER BY q.created_at ASC
3451
LIMIT 1
3552
-- Skip any that are being synced by other workers.
3653
FOR UPDATE SKIP LOCKED
3754
)
38-
DELETE FROM contribution_diff_queue
39-
USING chosen_contribution
40-
WHERE contribution_diff_queue.contribution_id = chosen_contribution.contribution_id
41-
RETURNING chosen_contribution.contribution_id
55+
DELETE FROM causal_diff_queue
56+
USING chosen
57+
WHERE causal_diff_queue.from_causal_id = chosen.from_causal_id
58+
AND causal_diff_queue.to_causal_id = chosen.to_causal_id
59+
AND causal_diff_queue.from_codebase_owner = chosen.from_codebase_owner
60+
AND causal_diff_queue.to_codebase_owner = chosen.to_codebase_owner
61+
RETURNING chosen.from_causal_id, chosen.to_causal_id, chosen.from_codebase_owner, chosen.to_codebase_owner
4262
|]

‎src/Share/BackgroundJobs/Diffs/Types.hs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
module Share.BackgroundJobs.Diffs.Types (CausalDiffInfo (..)) where
2+
3+
import Share.IDs
4+
import Share.Postgres qualified as PG
5+
import Share.Postgres.IDs
6+
7+
data CausalDiffInfo = CausalDiffInfo
8+
{ fromCausalId :: CausalId,
9+
toCausalId :: CausalId,
10+
fromCodebaseOwner :: UserId,
11+
toCodebaseOwner :: UserId
12+
}
13+
14+
instance PG.DecodeRow CausalDiffInfo where
15+
decodeRow =
16+
CausalDiffInfo
17+
<$> PG.decodeField
18+
<*> PG.decodeField
19+
<*> PG.decodeField
20+
<*> PG.decodeField

‎src/Share/BackgroundJobs/Monad.hs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module Share.BackgroundJobs.Monad
55
withWorkerName,
66
runBackground,
77
withTag,
8+
withTags,
89
)
910
where
1011

@@ -29,7 +30,10 @@ withWorkerName :: Text -> Background a -> Background a
2930
withWorkerName name = localBackgroundCtx \ctx -> ctx {workerName = name}
3031

3132
withTag :: Text -> Text -> Background a -> Background a
32-
withTag key value = localBackgroundCtx \ctx -> ctx {loggingTags = Map.insert key value (loggingTags ctx)}
33+
withTag key value = withTags [(key, value)]
34+
35+
withTags :: [(Text, Text)] -> Background a -> Background a
36+
withTags tags = localBackgroundCtx \ctx -> ctx {loggingTags = Map.union (Map.fromList tags) (loggingTags ctx)}
3337

3438
instance Logging.MonadLogger Background where
3539
logMsg msg = do

‎src/Share/Contribution.hs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ data Contribution = Contribution
7777
status :: ContributionStatus,
7878
sourceBranchId :: BranchId,
7979
targetBranchId :: BranchId,
80+
sourceCausalId :: CausalId,
81+
targetCausalId :: CausalId,
8082
bestCommonAncestorCausalId :: Maybe CausalId,
8183
createdAt :: UTCTime,
8284
updatedAt :: UTCTime,
@@ -94,6 +96,8 @@ instance Hasql.DecodeRow Contribution where
9496
status <- PG.decodeField
9597
sourceBranchId <- PG.decodeField
9698
targetBranchId <- PG.decodeField
99+
sourceCausalId <- PG.decodeField
100+
targetCausalId <- PG.decodeField
97101
bestCommonAncestorCausalId <- PG.decodeField
98102
createdAt <- PG.decodeField
99103
updatedAt <- PG.decodeField

‎src/Share/Metrics.hs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ module Share.Metrics
1010
tickUserSignup,
1111
recordBackgroundImportDuration,
1212
recordDefinitionSearchIndexDuration,
13-
recordContributionDiffDuration,
13+
recordCausalDiffDuration,
1414
recordWebhookSendingDuration,
1515
)
1616
where
@@ -445,8 +445,8 @@ recordBackgroundImportDuration = timeActionIntoHistogram backgroundImportDuratio
445445
recordDefinitionSearchIndexDuration :: (MonadUnliftIO m) => m r -> m r
446446
recordDefinitionSearchIndexDuration = timeActionIntoHistogram definitionSearchIndexDurationSeconds (deployment, service)
447447

448-
recordContributionDiffDuration :: Clock.TimeSpec -> IO ()
449-
recordContributionDiffDuration startTime = do
448+
recordCausalDiffDuration :: Clock.TimeSpec -> IO ()
449+
recordCausalDiffDuration startTime = do
450450
endTime <- Clock.getTime Monotonic
451451
recordLatency contributionDiffDurationSeconds (deployment, service) startTime endTime
452452

0 commit comments

Comments
 (0)