Jackie-Jiang commented on code in PR #18931:
URL: https://github.com/apache/pinot/pull/18931#discussion_r3540513673
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConfigProperties.java:
##########
@@ -58,15 +58,10 @@ private StreamConfigProperties() {
* @deprecated because the property key is confusing (says size but is
actually rows). Use
* {@link StreamConfigProperties#SEGMENT_FLUSH_THRESHOLD_ROWS}
*
- * Row count flush threshold for realtime segments. This behaves in a
similar way for HLC and LLC. For HLC,
- * since there is only one consumer per server, this size is used as the
size of the consumption buffer and
- * determines after how many rows we flush to disk. For example, if this
threshold is set to two million rows,
- * then a high level consumer would have a buffer size of two million.
- *
- * For LLC, this size is divided across all the segments assigned to a given
server and is set on a per segment
- * basis. Assuming a low level consumer server is assigned four stream
partitions to consume from and a flush
- * size of two million, then each consuming segment would have a flush size
of five hundred thousand rows, for a
- * total of two million rows in memory.
+ * Row count flush threshold for realtime segments. This size is divided
across all the segments assigned to a given
+ * server and is set on a per segment basis. Assuming a server is assigned
four stream partitions to consume from and
+ * a flush size of two million, then each consuming segment would have a
flush size of five hundred thousand rows, for
+ * a total of two million rows in memory.
Review Comment:
The longest line in this rewritten Javadoc block is exactly 120 characters
(the "...five hundred thousand rows, for" line), which is within checkstyle's
`LineLength` max of 120 — line 64 itself is only 43 characters. No wrapping
needed here; checkstyle passes.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConsumerFactory.java:
##########
@@ -62,30 +61,24 @@ public StreamPartitionMsgOffsetFactory
createStreamMsgOffsetFactory() {
return new LongMsgOffsetFactory();
}
- /**
- * Creates a partition group consumer, which can fetch messages from a
partition group
- */
- public PartitionGroupConsumer createPartitionGroupConsumer(String clientId,
- PartitionGroupConsumptionStatus partitionGroupConsumptionStatus) {
- return createPartitionLevelConsumer(clientId,
partitionGroupConsumptionStatus.getStreamPartitionGroupId());
- }
+ /// Creates a [PartitionGroupConsumer] that fetches messages from the
partition group described by
+ /// `partitionGroupConsumptionStatus`.
+ ///
+ /// Every [StreamConsumerFactory] implementation must override this method.
The returned consumer is owned by
+ /// the caller, which is responsible for closing it via
[PartitionGroupConsumer#close()]. Each invocation returns
+ /// a new consumer instance; implementations are not required to make the
returned consumer thread-safe.
+ ///
+ /// @param clientId identifies the creator of this consumer
+ /// @param partitionGroupConsumptionStatus the partition group to consume
and the offsets to start from
+ /// @return a new, non-null partition group consumer
+ public abstract PartitionGroupConsumer createPartitionGroupConsumer(String
clientId,
+ PartitionGroupConsumptionStatus partitionGroupConsumptionStatus);
Review Comment:
This is intentional. HLC has been unsupported since ~v0.12 and these APIs
were `@Deprecated`; the old default delegated to
`createPartitionLevelConsumer`, which is being removed, so keeping a concrete
default isn't meaningful. Making the method abstract gives compile-time
enforcement for all implementations (every in-tree factory already overrides
it). The removal is being shipped as a declared backward-incompatible SPI
change (`backward-incompat` label + migration note: external
`StreamConsumerFactory` implementations must implement
`createPartitionGroupConsumer`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
