Skip to content

major: New IPC system using lightweight Lambda actor queue - #325

Closed
Antony Stubbs (astubbs) wants to merge 196 commits into
confluentinc:masterfrom
astubbs:improvements/lambda-actor-bus
Closed

major: New IPC system using lightweight Lambda actor queue#325
Antony Stubbs (astubbs) wants to merge 196 commits into
confluentinc:masterfrom
astubbs:improvements/lambda-actor-bus

Conversation

@astubbs

@astubbs Antony Stubbs (astubbs) commented Jul 3, 2022

Copy link
Copy Markdown
Contributor
  • Documentation (if applicable)
  • Changelog
  • Review

- CVE-2020-8908: Files::createTempDir local information disclosure vulnerability #4011
- Only used transitively from tests, and is a deprecated function
- google/guava#4011
…-actor-bus

# Conflicts:
#	parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/AbstractParallelEoSStreamProcessor.java
…-actor-bus

# Conflicts:
#	parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/AbstractParallelEoSStreamProcessor.java
#	parallel-consumer-core/src/test/resources/logback-test.xml
Under unrealistically high load with no-op processing, broker poller unblocking a partition could cause ProcessingShard to skip forward in its entries and take work out of order.
Was discovered when fixing a synthetic high performance benchmark, after an O(n) algo was fixed to O(1), creating the state for the race condition to appear. Probably could not happen without the fix, as it's related to the performance of certain parts of the system.
Under unrealistically high load with no-op processing, broker poller unblocking a partition could cause ProcessingShard to skip forward in its entries and take work out of order.
Was discovered when fixing a synthetic high performance benchmark, after an O(n) algo was fixed to O(1), creating the state for the race condition to appear. Probably could not happen without the fix, as it's related to the performance of certain parts of the system.
… improvements/lambda-actor-bus

# Conflicts:
#	parallel-consumer-core/src/test/java/io/confluent/csid/utils/KafkaTestUtils.java
@eddyv

Copy link
Copy Markdown
Contributor

Closing - Stale.

Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Jul 28, 2026
* docs: add refactoring backlog (deferred internal refactors)

A versioned, grouped-by-file list of refactors too big/risky to fold into the
change at hand - captured for quiet time instead of GitHub issues (overkill for a
solo maintainer, and keeps the fork tracker signal). Cross-cutting/architectural
items (thread model / poll+control merge, God-class split, static-state removal,
actor-IPC) link to upstream #200/confluentinc#488/confluentinc#524/confluentinc#325/confluentinc#405 and the abandoned draft
branches as design refs rather than being rewritten. Seeded from a code scan
(TODO/FIXME + large-class signals) and a branch/issue/prior-PR sweep.

AGENTS.md documents the // TODO(refactor): marker convention, the doc boundaries
(vs inflight / upstream-map / solutions / PR review), and the graduation rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(refactoring): enumerate abandoned branches + prior drafts as a real index

Stop brushing over the pointers. Each abandoned refactor branch now has a
specific one-liner (what it did, relevance, linked issue/PR) grouped by theme:
thread-model/actor cluster (upstream #200), static-state removal, shard-count
caching perf (confluentinc#530), engine/queue experiments (confluentinc#884), encoding, offsets/state
classes (#233), API/interface, test infra. Records dead-ends explicitly (e.g.
producer-facade, whose branch concluded it was not worthwhile) and supersessions
(loom -> upstream confluentinc#908).

The bulk verdicts for the ~53 prior closed PRs stay in upstream-pr-analysis.adoc;
this index keeps the actionable pointers with issue links. Also added the two
other synchronized(this) lock-hygiene sites (ProducerManager.syncBeginTransaction,
DynamicLoadFactor.doStep) surfaced while checking the PR #57 fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(refactoring): address PR #67 review

- Add a dedicated "Breaking changes queued for next major version" section
  (release-gated, kept apart from the non-breaking refactors), consolidating
  the deprecated commitInterval/options removals, JStream removal, enum rename,
  and nonnull/module-info candidates. Slim the by-file entries to back-references.
- Pin every abandoned origin/<branch> reference to its current short SHA so the
  catalogue survives future branch cleanup; expand the brace-compressed lists.
- Disambiguate bare upstream refs that collide with fork branch names
  (confluentinc#857/confluentinc#859/confluentinc#908 -> upstream #NN) and note the convention that branch names
  encode the upstream number.
- Explain why so many draft branches exist: the fork was a personal experiment
  space before becoming the project's primary; mine them for ideas, then prune.
- AGENTS.md: mention the new release-gated breaking-changes section.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Aug 5, 2026
All 78 upstream issues are mirrored, diagnosed and backlinked, and 7 are closed
against a released version. It ran in a day, by hand and with parallel
subagents, so Phase 1's script was never written.

Four things went wrong and are recorded so the next person does not repeat them:

A GREP SWEEP IS NOT A DIAGNOSIS. The first "is this already implemented?" pass
probed for strings related to each issue and reported 11 as implemented; at
least 9 were false, because it found the code the issue was ABOUT - the log line
being complained about, a class merely mentioned - and read presence as a fix.
Discarded and redone by reading each issue. The probes that did work shared one
property: the string could only exist if the request had been implemented.

UPSTREAM'S "FIXED BY" ANNOTATIONS ARE UNRELIABLE. confluentinc#346, confluentinc#270, confluentinc#325, #181, #179,
#197 are all closed with mergedAt=null - around 40 of 53 PRs were closed on
2023-06-15 when the maintainer left, an administrative sweep rather than review
outcomes. Seven mirrors now carry an "Unmerged work" section saying so.

DO NOT TRUST AN ISSUE'S ACCOUNT OF WHAT IS PUBLISHED. Three mirrors repeated
that 0.5.3.3 was never published - true when written, false by the time we said
it. It reached Central on 2025-08-28. Check the artefact repository, not the
conversation.

AUTO-CLOSE IS SILENTLY FRAGILE. "Fixes #NNN" only closes on PRs targeting the
default branch, and a body rewrite silently dropped one. Verify with
closingIssuesReferences.

Also records what worked: fan-out for diagnosis with a single voice for writing,
notifying only when actionable (four reporters got usable answers), and that
diagnosing turned up six bugs nobody had filed.

Label scheme updated to match reality: enhancement -> feature, ver:1.0 -> 1.0,
plus fixed-in/, partially-fixed-in/, affects/, pr-available, and
next-feature-release / next-breaking-release instead of committing to version
numbers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Aug 5, 2026
My own gate failed my own PR, which is the useful kind of failure. Three real
defects and one design flaw, all surfaced by the CI run.

DESIGN FLAW. It compared each ref against "the fork's highest issue number",
read from the API. That is a proxy for the real question and it RACED: CI read
196 while #197 already existed, so a valid fork reference was flagged. Worse, a
threshold fails OPEN as the repo grows - once fork numbering passes upstream's
~920, every upstream reference reads as a valid fork ref and the gate silently
stops working, which is the collision this plan already predicts elsewhere.

It now asks whether #NN resolves in this repo, cached. That is the actual
question, cannot race, does not degrade, and additionally catches a typo'd
reference that a threshold below max would wave through. One lookup per distinct
number on added lines - a handful per PR - and a non-404 error counts as
"exists", so an API wobble fails open rather than blocking a PR falsely.

MARKDOWN LINKS. `[#233](https://github.com/confluentinc/.../233)` was flagged:
the URL qualifies it, but URL-stripping ran first and left a bare `[#233]`.
Links are now stripped whole, before their targets.

ITS OWN TEST FIXTURES. issue-ref-gate.test.js is deliberately full of
unqualified upstream refs and the gate flagged every one. Now exempt.

The remaining hits were genuine and are fixed in the docs. One deserves calling
out: the plan said "confluentinc#325, #181, #179 and #197 are all closed with mergedAt=null"
- that #197 is UPSTREAM's, and this fork now has its own #197, the release
issue. A reader following it lands on something unrelated. Exactly the collision
the convention exists to prevent, found by the gate written to enforce it.

13 assertions, including regressions for the race, the markdown-link case and
the newest-issue case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Antony Stubbs (astubbs) added a commit to astubbs/parallel-consumer that referenced this pull request Aug 5, 2026
…asings

The gate's second run found seven more, all genuine. Fixed by linking rather
than by qualifying in prose, and by using the fork mirror id where one exists.

WHY THE LIST FORM WAS ACTUALLY BROKEN. "upstream #233 / confluentinc#326 / confluentinc#857" looks
qualified but is not: GitHub links only the first: the trailing two render as
links to THIS repo. confluentinc#326 and confluentinc#857 will one day exist here and mean something
else. Those become the mirrors themselves - #117, #118 and #119 - which auto-link
correctly and are what a reader here should open anyway.

Upstream PRs have no mirror, since PRs are deliberately not mirrored, so they are
hyperlinked instead: the closed-unmerged list (confluentinc#346, confluentinc#270, confluentinc#325, #181, #179,
#197) and the "Fixed by confluentinc#346" quote in refactoring.md now point at the upstream
PRs they name.

GATE. It now accepts "upstream PR #N" and "upstream issue #N" as qualified -
natural phrasing it was wrongly flagging - while still flagging trailing items in
a list, with a test pinning exactly that, since that is the case a reader gets
wrong. 15 assertions.

Verified against the real diff rather than by eye: parsing `git diff` through the
gate module reports no flags on added lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
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

Successfully merging this pull request may close these issues.

2 participants