Source code
package org.apache.kafka.coordinator.share;
import org.apache.kafka.server.share.SharePartitionKey;
import org.apache.kafka.timeline.SnapshotRegistry;
import org.apache.kafka.timeline.TimelineHashMap;
import org.apache.kafka.timeline.TimelineLong;
import java.util.Objects;
import java.util.Optional;
/**
* Util class to track the offsets written into the internal topic
* per share partition key.
* It calculates the minimum offset globally up to which the records
* in the internal partition are redundant i.e. they have been overridden
* by newer records.
*/
public class ShareCoordinatorOffsetsManager {
// Map to store share partition key => current partition offset
// being written.
private final TimelineHashMap<SharePartitionKey, Long> offsets;
// Minimum offset representing the smallest necessary offset
// across the internal partition (offsets below this are redundant).
// We are using timeline object here because the offsets which are passed into
// updateState might not be committed yet. In case of retry, these offsets would
// be invalidated via the snapshot registry. Hence, using timeline object
// the values would automatically revert in accordance with the last committed offset.
private final TimelineLong lastRedundantOffset;
public ShareCoordinatorOffsetsManager(SnapshotRegistry a) {
Objects.requireNonNull(a);
offsets = new TimelineHashMap<>(a, 0);
lastRedundantOffset = new TimelineLong(a);
lastRedundantOffset.set(Long.MAX_VALUE); // For easy application of Math.min.
}
/**
* Method updates internal state with the supplied offset for the provided
* share partition key. It then calculates the minimum offset, if possible,
* below which all offsets are redundant.
*
* @param key - represents {@link SharePartitionKey} whose offset needs updating
* @param offset - represents the latest partition offset for provided key
* @param isDelete - true if the offset is for a tombstone record
*/
public void a(SharePartitionKey b, long c, boolean d) {
lastRedundantOffset.set(Math.min(lastRedundantOffset.get(), c));
offsets.put(b, c);
Optional<Long> e = b();
e.ifPresent(lastRedundantOffset::set);
// If the share partition is deleted, we should not hold onto its offset in our calculations
// as there is nothing beyond deletion which is going to update its state.
if (d) {
offsets.remove(b);
}
}
private Optional<Long> b() {
if (offsets.isEmpty()) {
return Optional.empty();
}
long f = Long.MAX_VALUE;
for (long g : offsets.values()) {
// Get min offset among latest offsets
// for all share keys in the internal partition.
f = Math.min(f, g);
// lastRedundantOffset represents the smallest necessary offset
// and if soFar equals it, we cannot proceed. This can happen
// if a share partition key hasn't had records written for a while.
// For example,
// <p>
// key1:1
// key2:2 4 6
// key3:3 5 7
// <p>
// We can see in above that offsets 2, 4, 3, 5 are redundant,
// but we do not have a contiguous prefix starting at lastRedundantOffset
// and we cannot proceed.
if (f == lastRedundantOffset.get()) {
return Optional.of(f);
}
}
return Optional.of(f);
}
/**
* Most recent last redundant offset. This method is to be used
* when the caller wants to query the value of such offset.
* @return Optional of type Long representing the offset or empty for invalid offset values
*/
public Optional<Long> c() {
long h = lastRedundantOffset.get();
if (h <= 0 || h == Long.MAX_VALUE) {
return Optional.empty();
}
return Optional.of(h);
}
// visible for testing
TimelineHashMap<SharePartitionKey, Long> d() {
return offsets;
}
}
Run 1
The code has a logical structure and comments explaining intent, but the use of single-letter method and variable names (`a`, `b`, `c`, `d`, `e`, `f`, `g`, `h`) severely hurts readability and obscures purpose. The comments describe what the methods do, yet the method names do not convey any meaning. Additionally, the constructor parameter `a` is poorly named. Although the code is mostly flat, the loop inside method `b()` with its early return and inline comments is somewhat hard to follow. The action granularity is mostly one action per line, but the first line of method `a()` performs two operations (set then get) and could be clearer. Overall, the code requires constant reference to comments to understand intent, which is not ideal for maintainability.
Run 2
The code suffers from extremely poor readability due to obfuscated method and variable names (a, b, c, d, e, f, g, h). While the class has a descriptive comment, the method names provide zero indication of their purpose, forcing readers to reverse-engineer intent. Parameter names like 'a' for SnapshotRegistry and 'b' for SharePartitionKey obscure meaning despite the import statements. The algorithm in method b() is nested within a loop with conditional logic that is difficult to follow due to single-letter variables. The code length is reasonable and the structure includes logical sections, but naming choices negate these positives. Comments explain what the code does, but the actual code does not self-document. Action granularity is adequate, and reading flow is disrupted primarily by the cryptic identifiers. This code would require significant cognitive overhead and constant reference to documentation or call sites to understand, making it nearly unreadable in practice.
Run 3
The code suffers from extremely poor naming conventions which severely hamper readability. Methods and variables use meaningless names like 'a', 'b', 'c', 'd', 'e', 'f', 'g', and 'h' instead of descriptive names reflecting their purpose (e.g., 'updateState', 'calculateMinOffset', 'getLastRedundantOffset', 'getOffsetsMap'). While the overall structure has some logical organization with comments explaining intent, the obfuscated naming contradicts these comments and makes the code ambiguous and difficult to follow. The code is relatively flat with moderate nesting, and generally one action per line, which helps somewhat, but the disastrous naming is a critical failure. The reading flow is unnatural because the reader must constantly refer back to comments to decipher what each single-letter identifier represents. Code length is reasonable, and the logical separation into methods is okay, but clarity of intent is essentially zero due to the naming. Overall, this code is barely maintainable.
Run 1
Code Structure: The class is logically organized into state, initialization, updates, minimum-offset calculation, querying, and test access. Nesting: Shallow loops and conditionals, with early returns, keep control flow easy to follow. Clarity of intent: Descriptive field names and explanatory comments help, but single-letter method and parameter names severely obscure operations. Documentation references parameter names and updateState that no longer match the code, adding confusion. Code length: The implementation is reasonably concise, although lengthy comments outweigh some simple operations. Action granularity: Statements generally perform one action per line, making state changes easy to inspect. Reading flow: Method boundaries and spacing support a natural progression, but cryptic calls such as b() and unexplained update-before-delete ordering require extra interpretation. Overall, sound structural readability is undermined primarily by poor naming and inconsistent documentation.
Run 2
Code Structure: The class logically separates initialization, updates, minimum-offset calculation, querying, and test access. Nesting: Shallow loops and conditionals with early returns keep control flow manageable. Clarity of intent: Descriptive field names and comments explain the domain, but single-letter method and variable names obscure operations and data roles. Javadoc parameter names do not match the actual signature, and comments reference nonexistent names such as updateState and soFar. Code length: Implementation is reasonably concise, though lengthy comments outweigh some simple operations. Action granularity: Statements generally perform one action per line, with conventional, readable expressions. Reading flow: Method ordering and whitespace support sequential reading, but cryptic identifiers force readers to repeatedly infer meaning from comments and surrounding code.
Run 3
Code Structure: Fields, constructor, update logic, minimum calculation, and accessors are logically organized. Nesting: Shallow conditionals and a single loop keep control flow manageable. Clarity of intent: Descriptive class and field names help, but single-letter method and variable names severely obscure operations and data roles. Javadoc parameter names do not match the actual signature, and comments reference outdated names, adding confusion. Code length: Implementation is reasonably concise, though lengthy comments outweigh some simple operations. Action granularity: Statements generally perform one action per line, making individual steps easy to follow. Reading flow: Formatting and early returns support sequential reading, but cryptic calls such as b() and unexplained sentinel conventions require readers to reconstruct meaning from surrounding comments.