Skip to content

commitOffsets can be passed the offsets to commit#10

Closed
squito wants to merge 2 commits intoapache:0.8from
squito:commitOffsets_param
Closed

commitOffsets can be passed the offsets to commit#10
squito wants to merge 2 commits intoapache:0.8from
squito:commitOffsets_param

Conversation

@squito
Copy link
Copy Markdown

@squito squito commented Nov 25, 2013

This adds another version of commitOffsets that takes the offsets to commit as a parameter.

Without this change, getting correct user code is very hard. Despite kafka's at-least-once guarantees, most user code doesn't actually have that guarantee, and is almost certainly wrong if doing batch processing. Getting it right requires some very careful synchronization between all consumer threads, which is both:

  1. painful to get right
  2. slow b/c of the need to stop all workers during a commit.

This small change simplifies a lot of this. This was discussed extensively on the user mailing list, on the thread "are kafka consumer apps guaranteed to see msgs at least once?"

You can also see an example implementation of a user api which makes use of this, to get proper at-least-once guarantees by user code, even for batches:
https://github.com/quantifind/kafka-utils/pull/1

I'm open to any suggestions on how to add unit tests for this.

@lazyval
Copy link
Copy Markdown
Contributor

lazyval commented Nov 25, 2013

@squito AFAIK code improvements are only accepted in a form of jira ticket + attached patch file. Not absolutely sure what is the official policy, but looks like this mirror is kept only for code navigation/personal forks purpose.

@squito
Copy link
Copy Markdown
Author

squito commented Nov 25, 2013

thanks @lazyval . I've created this ticket https://issues.apache.org/jira/browse/KAFKA-1144 .

@squito squito closed this Nov 25, 2013
@lazyval lazyval mentioned this pull request Jan 14, 2014
ymatsuda referenced this pull request in confluentinc/kafka Aug 5, 2015
ymatsuda pushed a commit to ymatsuda/kafka that referenced this pull request Aug 27, 2015
Parth-Brahmbhatt pushed a commit to Parth-Brahmbhatt/kafka that referenced this pull request Nov 3, 2015
BUG-45592: Migration script for Upgrading old acls to new acls.
xiowu0 pushed a commit to xiowu0/kafka that referenced this pull request Apr 2, 2019
…s Java objects as arguments. (apache#10)

Reviewers: Radai Rosenblatt, Ke Hu
abhishekmendhekar pushed a commit to abhishekmendhekar/kafka that referenced this pull request Jun 12, 2019
…ce only takes Java objects as arguments. (apache#10)

TICKET =
LI_DESCRIPTION =

Reviewers: Radai Rosenblatt, Ke Hu

EXIT_CRITERIA = MANUAL ["describe exit criteria"]
efeg pushed a commit to efeg/kafka that referenced this pull request Jan 29, 2020
…md (apache#10)

Clean up metrics aggregator. 
Improve quick start procedure in README.md
Fixed a bug in the MetricCompletenessChecker
lianetm added a commit to lianetm/kafka that referenced this pull request Jun 2, 2023
* beginningOffsets and endOffsets implementation

* ListOffsetsRequestManager implementation & event processing integration
lianetm added a commit to lianetm/kafka that referenced this pull request Jun 12, 2023
* beginningOffsets and endOffsets implementation

* ListOffsetsRequestManager implementation & event processing integration
brandboat pushed a commit to brandboat/kafka that referenced this pull request Sep 15, 2025
…tadata-manager

Add remote cluster metadata manager
haianh1233 added a commit to haianh1233/kafka that referenced this pull request Apr 17, 2026
New tests:
- apache#8  Port conflict (HTTP == PLAINTEXT port) → rejected
- apache#9  HTTP port=0 (random) works
- apache#10 HTTP + HTTPS coexist on same broker
- apache#11 advertised.listeners with HTTP parsed correctly
- apache#12 HTTP without httpAcceptorFactory → IllegalStateException
- apache#13 inter.broker.listener=HTTPS also rejected (not just HTTP)
- apache#14 Custom listener name mapped to HTTP protocol (MY_REST_API:HTTP)
- apache#15 HTTPS with valid SSL config succeeds

All 15 SocketServerHttpTest pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Raikion201 added a commit to Raikion201/kafka that referenced this pull request Apr 17, 2026
Issue apache#4 — Quota entity serialization:
  Sort entity keys alphabetically before joining with "|" separator,
  so (user+client-id) always serializes identically regardless of
  HashMap iteration order. Previously could produce different strings
  for the same entity, causing silent duplicates.

Issue apache#5 — Hardcoded credentials:
  SecurityStoreConfig.resolveValue() now supports ${VAR} and ${VAR:-default}
  syntax for environment variable substitution. Updated server.properties
  to use ${KAFKA_SECURITY_STORE_USER:-kafka} and
  ${KAFKA_SECURITY_STORE_PASSWORD:-kafka}.

Issue apache#6 — Byte cast:
  Use rs.getShort() to match SMALLINT column type, then cast to byte.
  Previous getInt() + (byte) cast had overflow risk in theory.

Issue apache#7 — MetadataLoader silent partial load:
  injectSecurityData() now throws on failure instead of silently
  returning stale image. Prevents users from retaining revoked access.

Issue apache#10 — apiKey filter comments:
  Extracted SECURITY_STORE_API_KEYS constant with inline comments
  identifying each record type. Replaced boolean chain with Set.contains().

Issue apache#11 — persistUpsertion fragility:
  Now searches records by (name, mechanism) instead of assuming the
  target is always the last record. Throws IllegalStateException
  if no matching record is found.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Raikion201 added a commit to Raikion201/kafka that referenced this pull request Apr 17, 2026
Issue apache#4 — Quota entity serialization:
  Sort entity keys alphabetically before joining with "|" separator,
  so (user+client-id) always serializes identically regardless of
  HashMap iteration order.

Issue apache#5 — Hardcoded credentials:
  SecurityStoreConfig.resolveValue() now supports ${VAR} and
  ${VAR:-default} syntax for environment variable substitution.
  server.properties updated to use env vars by default.

Issue apache#6 — Byte cast:
  Use rs.getShort() to match SMALLINT column type, then cast to byte.

Issue apache#7 — MetadataLoader silent partial load:
  injectSecurityData() now throws on failure instead of silently
  returning stale image.

Issue apache#10 — apiKey filter comments:
  Extracted SECURITY_STORE_API_KEYS constant with inline comments.

Issue apache#11 — persistUpsertion fragility:
  Searches records by (name, mechanism) instead of assuming
  the target is last.

Also add .gitignore entries for .metals/, .bloop/, and runtime logs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blitzy Bot pushed a commit to blitzy-public-samples/blitzy-kafka that referenced this pull request Apr 18, 2026
Resolve all 9 Minor and 10 Info findings from the Checkpoint 1 code review,
correcting factual inaccuracies, citation line-range imprecisions, and cross-
artifact consistency drift. No modifications to pre-existing Kafka source,
tests, build files, or comments — Audit Only rule preserved.

Findings by file:

accepted-mitigations.md
  #1 [MINOR] AclCache imports corrected: org.apache.kafka.server.immutable
              (PCollections-backed Kafka-internal) instead of Guava's
              com.google.common.collect.
  apache#2 [MINOR] API surface rewritten to reflect PCollections-style structural-
              sharing methods .updated()/.added()/.removed() instead of
              Guava builder pattern.
  apache#3 [MINOR] ZstdCompression BufferPool path split: wrap-for-output uses
              zstd-jni RecyclingBufferPool.INSTANCE (L55-L63), wrap-for-
              input uses ChunkedBytesStream (L65-L75), wrap-for-zstd-input
              uses anonymous Kafka-owned BufferPool delegating to
              BufferSupplier (L77-L98).
  apache#4 [INFO]  MAX_RECORDS_PER_USER_OP citation corrected: declaration at
              QuorumController.java:L185; AclControlManager.java:L52 is
              the static import only.
  apache#5 [INFO]  AclCache.removeAcl(Uuid) line corrected to L91-L103 (was L89+).

references.md
  apache#6 [MINOR] SafeObjectInputStream citation range tightened from L17-L25
              (class header + imports only) to L25-L62 covering the class
              declaration, DEFAULT_NO_DESERIALIZE_CLASS_NAMES blocklist
              (L27-L37), resolveClass (L43-L52), and isBlocked helper
              (L54-L62).
  apache#7 [INFO]  PropertyFileLoginModule citation corrected to L42-L50,
              pointing at the Javadoc PLAINTEXT warning (L47-L48) plus
              the class declaration (L50).

remediation-roadmap.md
  apache#8 [INFO]  Gantt markers sanitised: all :done/:active markers replaced
              with :crit (illustrative critical emphasis) or plain markers
              to avoid any visual suggestion of work already performed.
              Explanatory blockquote added clarifying the marker change.

severity-matrix.md
  apache#9 [MINOR] 7 occurrences of parenthesised '(Accepted Mitigation)'
              replaced with bracketed '[Accepted Mitigation]' per Global
              Conventions for plain-text markers. Cross-validated 9
              bracketed instances, 0 parenthesised remaining.

README.md
  apache#11 [MINOR] HEAD commit reference corrected to the pre-audit baseline
               6d16f68 (was 8a99096, a
               mid-audit snapshot); baseline attestation now refers to the
               commit immediately before the audit began.
  apache#12 [MINOR] Snapshot date unified to 2026-04-17 across all artifacts.
  apache#14 [INFO]  '25 files' claim qualified as 'planned at project completion'
               vs 'delivered at this checkpoint (15 files)'.

attack-surface-map.md
  apache#16 [MINOR] Clients module category count corrected from 'six' to 'nine'
               (actual Mermaid edges: C1, C2, C3, C4, C5, C7, C8, C9, C10).
  apache#17 [MINOR] Connect module category count corrected from 'five' to
               'seven' (actual Mermaid edges: C1, C4, C6, C7, C8, C9, C10).

oauth-jwt-validation-paths.md
  apache#18 [INFO]  Outer citation ranges tightened:
               BrokerJwtValidator.configure at L107-L138 (not L102-L134);
               OAuthBearerUnsecuredValidatorCallbackHandler.handleCallback
               at L154-L177 (not L161-L204, which spanned unrelated
               helpers); allowableClockSkewMs helper cited separately at
               L194-L207.

executive-summary.html
  Cross-ref A [MINOR] HEAD commit aligned to 6d16f68 at three sites
                       (L621, L668, L1544); methodology Mermaid node
                       re-labelled 'Baseline 6d16f68'.
  Cross-ref B [MINOR] Snapshot date aligned to 2026-04-17 at two sites
                       (L619, L1542).

Out-of-scope (Info-level forward-refs):
  apache#10, apache#13, apache#15 — Links to docs/security-audit/findings/*.md deliverables
                   not yet present at Checkpoint 1; expected per scope
                   boundary; will resolve at Checkpoint 2 when the 10
                   per-category findings files land.

Validation results (Phase 3):
  - Mermaid fences: all balanced (20 blocks total, all typed)
  - HTML tag balance: 22 sections + all 20+ tag types balanced
  - CDNs intact: reveal.js 5.1.0, Mermaid 11.4.0, Font Awesome 6.6.0
  - Emojis: zero across all 15 artifacts
  - TODOs/placeholders introduced: zero
  - Gantt markers: :crit + plain only (no :done/:active)
  - Cross-artifact consistency: zero wrong SHA/date values remaining
  - Citation ranges: 12 verified against AclCache, QuorumController,
                     AclControlManager, ZstdCompression,
                     SafeObjectInputStream, PropertyFileLoginModule,
                     BrokerJwtValidator, and
                     OAuthBearerUnsecuredValidatorCallbackHandler.

Audit Only rule verification:
  git diff --name-status 6d16f68..HEAD returns only 'A' entries,
  all under docs/security-audit/. Zero modifications, deletions, or
  renames of any pre-existing Kafka path.
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