Skip to content

Commit 3cfddbc

Browse files
committed
docs: add github app improvement design spec
1 parent 7ac4bc9 commit 3cfddbc

2 files changed

Lines changed: 340 additions & 0 deletions

File tree

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# GitHub App 개선 Phase 1 + 2 Pseudo Code
2+
3+
리뷰용 문서.
4+
문서에서 Anchor는 GitHub에 달리는 코멘트를 뜻한다.
5+
6+
## 1. EventWorker
7+
8+
```go
9+
func (w *EventWorker) ProcessPacket(ctx context.Context, msg QueuedPacket) error {
10+
pkt := msg.Packet
11+
subjectKey, ok := computeSubjectKey(pkt)
12+
if !ok {
13+
return w.queue.Ack(ctx, msg.StreamID)
14+
}
15+
16+
return w.locker.WithSubjectLock(ctx, subjectKey, func() error {
17+
state, err := w.state.Load(ctx, subjectKey)
18+
if err != nil {
19+
return err
20+
}
21+
22+
if hasDispatchableRoot(state) {
23+
return w.dispatch.DispatchCurrent(ctx, msg, state, pkt)
24+
}
25+
26+
if requiresExistingRoot(pkt) {
27+
rebuilt, ok, err := w.tryResolveExistingRoot(ctx, subjectKey, pkt)
28+
if err != nil {
29+
return err
30+
}
31+
if ok {
32+
return w.dispatch.DispatchCurrent(ctx, msg, rebuilt, pkt)
33+
}
34+
return w.queue.Ack(ctx, msg.StreamID)
35+
}
36+
37+
if rebuilt, ok, err := w.tryLazyRebuildFromVerifiedAnchor(ctx, subjectKey, pkt); err != nil {
38+
return err
39+
} else if ok {
40+
return w.dispatch.DispatchCurrent(ctx, msg, rebuilt, pkt)
41+
}
42+
43+
if shouldBufferUntilRoot(pkt, state, w.clock.Now()) {
44+
_, err := w.preRoot.AppendIfNewDelivery(ctx, subjectKey, pkt.DeliveryID, pkt)
45+
if err != nil {
46+
return err
47+
}
48+
49+
waiting := ensureWaitingRootState(subjectKey, state, w.clock.Now())
50+
if err := w.state.Save(ctx, waiting); err != nil {
51+
return err
52+
}
53+
return w.queue.Ack(ctx, msg.StreamID)
54+
}
55+
56+
if shouldMarkWaitingRootExpired(state, w.clock.Now()) {
57+
expired := ensureWaitingRootExpiredState(subjectKey, state, w.clock.Now())
58+
if err := w.state.Save(ctx, expired); err != nil {
59+
return err
60+
}
61+
state = expired
62+
}
63+
64+
root, ok, err := w.tryCreateRootIfEligible(ctx, subjectKey, state, pkt)
65+
if err != nil {
66+
return err
67+
}
68+
if !ok {
69+
return w.queue.Ack(ctx, msg.StreamID)
70+
}
71+
72+
if err := w.flushBufferedPreRootPackets(ctx, root); err != nil {
73+
return err
74+
}
75+
76+
if !isOpened(pkt) {
77+
return w.dispatch.DispatchCurrent(ctx, msg, root, pkt)
78+
}
79+
80+
root.LastProcessedAt = w.clock.Now()
81+
root.LastProcessedID = pkt.DeliveryID
82+
if err := w.state.Save(ctx, root); err != nil {
83+
return err
84+
}
85+
return w.queue.Ack(ctx, msg.StreamID)
86+
})
87+
}
88+
```
89+
90+
- dispatchable root가 있으면 바로 root-ready dispatch path로 보낸다.
91+
- `computeSubjectKey`는 final subject를 정할 수 없는 packet에 대해 `(subjectKey, false)` 를 반환할 수 있다.
92+
- 현재 기준으로 `status` / `check_run`은 merged PR association miss 시 여기서 바로 종료한다.
93+
- root가 없고 기존 root가 반드시 필요한 packet이면 current behavior와 동일한 existing-root lookup / bounded retry를 먼저 시도하고, 그 lookup이 실패한 경우에만 종료한다.
94+
- root가 없으면 verified Anchor -> pre-root queue -> `waiting_root_expired` -> eligible root 생성 순서로 내려간다.
95+
- pre-root append는 delivery 단위로 idempotent 해야 한다.
96+
- 같은 stream message가 worker retry로 재처리돼도 동일 `deliveryID` 는 한 번만 buffer append 되고, `waiting_root` state는 재시도에서 항상 복구 가능해야 한다.
97+
- `DispatchCurrent` 는 Phase 1/2에서 downstream write-side ledger를 두지 않는 best-effort dispatch seam이다.
98+
- 따라서 downstream thread write 성공 후 ack/state 저장 전에 worker가 중단되면 동일 `deliveryID` 의 thread message가 중복 전송될 수 있다.
99+
- `opened`가 wait window 안에 오면 그 packet으로 normal root를 만든다.
100+
- wait window가 지나면 즉시 synthetic fallback root를 만들지 않고 `waiting_root_expired` 로 전이하되, 그 상태 전이를 만든 현재 packet은 ack하지 않고 이어서 rebuild/fallback 판단에 사용한다.
101+
- 이후 같은 subject의 새 packet이 오면 lazy rebuild를 먼저 시도하고, 그마저 실패한 경우에만 fallback root를 한 번 생성한다.
102+
- root가 생기면 buffered packet과 current packet을 같은 dispatch seam으로 흘린다.

docs/improve-github-app/README.md

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
# GitHub App 개선 ValKey Schema 요약
2+
3+
용어 -> Goals / Non-goals / Semantics -> ValKey Schema.
4+
5+
## 1. 용어 정의
6+
7+
### 1-1. deliveryID
8+
9+
`deliveryID`: GitHub webhook request의 `X-GitHub-Delivery` header 값.
10+
11+
- transport-level identifier
12+
- packet 저장과 idempotency check 기준값
13+
- header가 없으면 정상 webhook으로 취급하지 않음
14+
15+
Phase 1 dedup key 정의:
16+
17+
```text
18+
deliveryID := req.Header["X-GitHub-Delivery"]
19+
dedupKey := "dedup:delivery:" + deliveryID
20+
```
21+
22+
### 1-2. subject
23+
24+
`subject`: Phase 1/2에서 하나의 채널톡 루트메시지와 채널톡 스레드 상태를 공유하는 canonical 처리 단위.
25+
26+
이 문서의 `subject/root/anchor` 모델은 issue / PR 계열에만 적용한다.
27+
release event는 현재 구현처럼 standalone group message로만 dispatch하며, Phase 1/2의 shared root state / Anchor recovery 범위에 포함하지 않는다.
28+
29+
canonical subject는 아래 두 종류로 고정.
30+
31+
```text
32+
issue:{org}/{repo}#{number}
33+
pr:{org}/{repo}#{number}
34+
```
35+
36+
- issue event -> `issue:{org}/{repo}#{number}`
37+
- pull_request 계열 event -> `pr:{org}/{repo}#{number}`
38+
- issue_comment on issue -> `issue:{org}/{repo}#{number}`
39+
- issue_comment on pull request -> `pr:{org}/{repo}#{number}`
40+
- status/check_run -> merged된 PR association hit 시 기존 `pr:{org}/{repo}#{number}` subject에만 매핑
41+
42+
worker가 이 값을 계산해 lock/state key 생성.
43+
44+
GitHub의 `issue_comment`는 issue/PR 공용 이벤트다.
45+
`Issue.IsPullRequest()==true` 인 경우 canonical subject는 항상 `pr:{org}/{repo}#{number}` 로 본다.
46+
47+
status/check_run 은 독립 canonical subject를 만들지 않는다.
48+
worker는 sha로 merged된 PR association을 조회하고, 연결된 PR이 있으면 해당 `pr` subject로만 처리한다.
49+
association miss 시 packet은 drop/skip 한다.
50+
51+
### 1-3. Anchor
52+
53+
`Anchor`: GitHub issue / PR에 앱이 남기는 코멘트.
54+
55+
- 채널톡 루트메시지 위치를 가리키는 보조 힌트
56+
- root state source-of-truth는 아니고, Phase 2 lazy rebuild와 retry 기준값으로 사용
57+
58+
## 2. Goals
59+
60+
- delivery dedup을 도입해 같은 webhook delivery의 중복 처리를 막는다.
61+
- issue / PR subject 단위 lock/shared state를 도입해 canonical root를 한 번만 정한다.
62+
- out-of-order event를 pre-root queue와 lazy rebuild로 흡수한다.
63+
- partial failure after root write 상황에서도 state/Anchor 기반으로 resume 가능하게 만든다.
64+
- `status` / `check_run`은 기존 merged PR thread에만 연결되게 유지한다.
65+
66+
## 3. Non-goals
67+
68+
- end-to-end exactly-once dispatch를 보장하지 않는다.
69+
- downstream thread write 성공 후 ack/state 저장 전 crash로 인한 duplicate thread message는 Phase 1/2 범위에서 제거하지 않는다.
70+
- late `opened`를 위해 root merge나 root replacement를 하지 않는다.
71+
- `status` / `check_run`용 독립 subject나 fallback root를 만들지 않는다.
72+
- release event를 Phase 1/2의 subject/root/anchor state machine에 포함하지 않는다.
73+
- ingress stream을 장기 source-of-truth 저장소로 쓰지 않는다.
74+
75+
## 4. Processing Semantics
76+
77+
- ingress stream consumption과 worker retry 모델은 `at least once`를 전제로 한다.
78+
- `dedup:delivery:{deliveryID}` 는 동일 delivery의 중복 진입을 줄이기 위한 guard이지, 모든 downstream side effect의 exactly-once를 의미하지 않는다.
79+
- ingress는 `dedup check + stream append + dedup record` 를 ValKey Lua/function 한 번으로 원자적으로 수행한다.
80+
- ingress는 atomic enqueue 결과를 받기 전에는 delivery를 성공 처리하지 않는다. atomic step 실패 시 request는 실패로 남기고 GitHub redelivery에 의존한다.
81+
- subject root selection은 strong invariant로 본다. 즉 `rootMessageId` 가 한 번 저장되면 그 root가 canonical root다.
82+
- pre-root queue는 bounded hold 용도다. packet은 짧은 window 동안만 유지하고, timeout 이후에는 `waiting_root_expired` 와 lazy rebuild로 복구를 시도한다.
83+
- downstream dispatch는 best-effort retry 대상이다. worker가 downstream thread write 성공 후 ack/state 저장 전에 중단되면 동일 delivery의 thread message가 중복 관찰될 수 있다.
84+
- 따라서 Phase 1/2의 dedup 범위는 ingress 진입, pre-root append, canonical root selection까지로 한정한다.
85+
86+
## 5. ValKey Schema
87+
88+
Schema는 phase별 책임과 필요한 키만 남김.
89+
90+
### 5-1. Phase 1 목표와 필수 키
91+
92+
목표: delivery dedup, subject lock/shared state, pre-root queue 도입.
93+
94+
```text
95+
# delivery dedup
96+
dedup:delivery:{deliveryID} -> "{streamEntryID}" TTL 7d
97+
98+
# ingress queue
99+
stream:github-events -> WebhookPacket retain 1h, trim by age/size
100+
101+
# subject lock
102+
lock:subject:{subjectKey} -> "{workerId}" TTL 30s
103+
104+
# subject state
105+
subject:{subjectKey} -> Hash / JSON TTL 90d, refresh on write
106+
107+
# pre-root queue
108+
pre-root:subject:{subjectKey} -> List(WebhookPacket) TTL 30s
109+
110+
# pre-root idempotency guard
111+
pre-root:buffered-delivery:{subjectKey} -> Set(deliveryID) TTL 30s
112+
```
113+
114+
키 역할:
115+
116+
- `dedup:delivery:{deliveryID}`
117+
- 같은 delivery를 두 번 처리하지 않기 위한 idempotency key
118+
- value는 `"1"` 고정값이 아니라 ingress가 enqueue에 성공했을 때 받은 `streamEntryID`
119+
- duplicate redelivery는 key 존재만으로 판별하고, 저장된 `streamEntryID` 는 운영 디버깅/추적에 사용
120+
- `stream:github-events`
121+
- ingress가 raw webhook packet을 적재하는 단일 stream
122+
- `PacketQueue`의 ValKey Streams backend
123+
- source-of-truth 저장소가 아니라 짧은 transport backlog이므로 최근 1시간만 보관
124+
125+
Ingress write contract:
126+
127+
```text
128+
atomicEnqueue(packet):
129+
if EXISTS dedup:delivery:{deliveryID}:
130+
return duplicate
131+
132+
streamEntryID = XADD stream:github-events * packet
133+
SET dedup:delivery:{deliveryID} streamEntryID EX 7d
134+
return enqueued(streamEntryID)
135+
```
136+
137+
- 위 세 단계는 ingress에서 분리 수행하지 않고 ValKey Lua/function 한 번으로 실행한다.
138+
- `SET dedup` 이 먼저 성공하고 `XADD` 전에 중단되는 경우, 혹은 `XADD``SET dedup` 전에 중단되는 경우를 허용하지 않는다.
139+
- 즉 ingress crash/retry에 대해 `deliveryID``stream:github-events` enqueue는 0회 또는 1회만 관찰되게 만든다.
140+
- `lock:subject:{subjectKey}`
141+
- 같은 subject를 동시에 두 worker가 처리하지 못하게 하는 distributed lock
142+
- `subject:{subjectKey}`
143+
- root를 다시 찾기 위한 minimal root pointer state를 저장하는 shared state
144+
- write/update 때 TTL을 90일로 갱신
145+
- 장기간 닫힌 subject는 자연 만료되고, state miss는 Phase 2 lazy rebuild로 복구
146+
- `pre-root:subject:{subjectKey}`
147+
- root가 아직 없는 동안 subject의 모든 packet을 arrival order대로 잠깐 보관
148+
- wait window보다 길게 유지해 timeout sweep가 늦어도 queue가 먼저 사라지지 않게 함
149+
- `opened` 도착 시 채널톡 루트메시지 생성 후 drain
150+
- `opened`가 wait window 안에 오지 않아도 즉시 synthetic fallback root를 만들지 않음
151+
- timeout 시 subject를 `waiting_root_expired` 로만 표시하고, 이후 같은 subject의 새 packet이 올 때 지연 복구를 시도
152+
- `pre-root:buffered-delivery:{subjectKey}`
153+
- pre-root queue에 이미 적재한 `deliveryID` 를 기록하는 idempotency guard
154+
- worker retry로 같은 packet이 재처리돼도 동일 `deliveryID` 는 한 번만 append 한다
155+
- `pre-root:subject:{subjectKey}` 와 같은 TTL로 관리한다
156+
157+
`subject:{subjectKey}` 최소 필드는 아래와 같이 고정.
158+
159+
```text
160+
subjectKey
161+
rootMessageId
162+
channelId
163+
groupId
164+
rootState # waiting_root | waiting_root_expired | pending_anchor | ready
165+
updatedAt
166+
```
167+
168+
`groupId`는 해당 채널톡 스레드 생성 시점의 groupId를 본다.
169+
config가 바뀌어도 기존 subject는 기존 `channelId/groupId/rootMessageId`를 계속 사용하고, 새 subject만 새 config 적용.
170+
171+
`stream:github-events`에 적재하는 packet는 ingress에서 final subject key를 확정하지 않음.
172+
worker가 final subject key를 계산할 수 있도록 routing hint와 raw payload 중심으로 저장.
173+
174+
```text
175+
deliveryID
176+
eventName
177+
action
178+
org
179+
repo
180+
number
181+
sha
182+
receivedAt
183+
headers
184+
payload
185+
```
186+
187+
`status` / `check_run`은 ingress 시점에 아직 merged PR association이 없을 수 있다.
188+
이 경우 별도 fallback subject를 만들지 않고 packet을 종료한다.
189+
190+
다만 merged PR association이 확인된 뒤에는 current behavior와 동일하게 기존 root comment(Anchor) 기반 existing-root lookup을 먼저 시도한다.
191+
이 경로의 root lookup miss는 immediate drop 대상이 아니고, 짧은 bounded retry/backoff 이후에도 root를 찾지 못한 경우에만 종료한다.
192+
193+
`opened` 누락 시 root 생성 정책은 아래와 같이 고정한다.
194+
195+
1. wait window 안에 `opened`가 오면 그 packet으로 normal root를 만든다.
196+
2. wait window가 지나도 `opened`가 없으면 즉시 synthetic fallback root를 만들지 않는다.
197+
3. 대신 `subject:{subjectKey}``waiting_root_expired` 상태로 남기고 pre-root packet은 TTL 동안 유지한다.
198+
4. 이후 같은 subject의 새 packet이 오면 worker는 먼저 existing root/state를 확인하고, 없으면 Anchor 기반 lazy rebuild를 시도한다.
199+
5. rebuild도 실패한 경우에만 그 시점의 packet을 기준으로 fallback root를 한 번 생성한다.
200+
6. `rootMessageId` 가 한 번 저장되면 그 root가 canonical root다. late `opened` 는 새 root를 만들지 않고 기존 root에 대한 일반 packet처럼 처리하거나 noop 한다.
201+
202+
즉 정책은 `first root wins forever` 이다.
203+
late `opened` reconciliation을 위해 root merge나 root 교체는 하지 않는다.
204+
205+
### 5-2. Phase 2 목표와 확장 키
206+
207+
목표: pending Anchor retry, verified Anchor lazy rebuild, bounded abandon.
208+
209+
`status` / `check_run`은 root 생성 트리거가 아니다.
210+
기존 merged PR root와 연결된 경우에만 thread message dispatch 대상으로 본다.
211+
merged PR association이 없으면 buffer/rebuild/fallback 없이 종료한다.
212+
다만 merged PR association hit 이후에는 current behavior와 동일하게 기존 root comment(Anchor) 기반 existing-root lookup을 먼저 시도한다.
213+
이 경로의 root lookup miss는 immediate drop 대상이 아니고, bounded retry/backoff 이후에도 root를 찾지 못한 경우에만 종료한다.
214+
215+
```text
216+
# pending anchor retry index
217+
anchor:pending-retry -> ZSET(subjectKey, retryAt) no TTL, remove on success + stale cleanup
218+
```
219+
220+
키 역할:
221+
222+
- `anchor:pending-retry`
223+
- GitHub에 달리는 코멘트(Anchor) 재시도가 필요한 subject 목록
224+
- `subject:{subjectKey}`
225+
- Anchor retry metadata와 마지막 실패 원인 저장
226+
- retry 성공 시 관련 필드 정리
227+
228+
권장 필드:
229+
230+
```text
231+
anchorRetryCount
232+
anchorLastError
233+
anchorLastTriedAt
234+
anchorNextRetryAt
235+
anchorCommentId
236+
```
237+
238+
이 키들은 Anchor retry worker가 소비한다.

0 commit comments

Comments
 (0)