If you roll log segments based on retention time, seems like you can have only one segment for that log at any point of time. If you want to roll 5 minute segments, it means that you can only have 5 minute worth of data for that partition. On the contrary, if I choose size based rolling and size based retention, I can have multiple log segments each of a specific size. What seems desirable is to have time based rolling + retention also behave the same way. I would imagine applications wanting to roll segments every 1 hour and retain 24 hours worth of data. This is an advantage for applications using getOffsetsBefore() to do some time indexed fetch of the data, since getOffsetsBefore only returns offsets at the log segment granularity. And it also gives applications a way to reason about the time window of the data retained for a partition. One potential downside is that, you can end up creating large number of log segments for your partition, if you choose too small a value for log.file.time.ms. But this problem exists today with size based log segment rolling too. So we are not introducing any regression in behavior.
Other review comments -
1.1 Rename currentMS to currentMs (Follow camel case convention).
1.2 How about renaming retentionMSInterval to retentionIntervalMs to be consistent with naming convention ?
1.3 In maybeRoll, looks like currentMS is unused apart from being used to compute the time difference. How about removing currentMS ?
2.1 This is unrelated to your patch, but lets also rename logRetentionMSMap to logRetentionMsMap