codelipenghui commented on issue #15073: URL: https://github.com/apache/pulsar/issues/15073#issuecomment-1094955483
The new introduced map `ConcurrentSkipListMap<PositionImpl, PositionImpl> metadataPositions` should be named `pendingAckLogReverseIndex`, I think this should be a more precise name. > upperLimitOfMaxAckPositionChangeTimes = metadataPositions.size * 500 I'm not fully understanding here, why need * 500 here, I think we can just add a configuration `transactionPendingAckLogReverseMaxIndexSize`? If reaching the max limitation, we only update the last element of the index, after the old element is deleted, we can continue to add a new element to the index map. And to avoid the `too close` indexes(to avoid creating an index for pending ack log positions 1 and 2), we can add a `transactionPendingAckLogMinLag`. > thisLastConfirmEntry: the position persistent by pendingAck log, when need to store a maxAckPosition. It's better not to use the LAC here(because LAC is a mutable value in Pulsar Concepts), it should be the position in the pendingAck log? -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
