Re: [VOTE] KIP-258: Allow to Store Record Timestamps in RocksDB

["Matthias J. Sax" <ma...@confluent.io>]

Another small change to the KIP:

We want to add two more public static helper methods `make()` and
`getValueOrNull` to simplify `null` handling. Also, the constructor of
`ValueAndTimestamp` should be private.


> public class ValueAndTimestamp<V> {
>     private ValueAndTimestamp(final V value, final long timestamp); // use `make()` instead>     public V value();
>     public long timestamp();
>     public static <V> ValueAndTimestamp<V> make(final V value, final long timestamp); // returns `null` if `value==null`
>     public static <V> V getValueOrNull(final ValueAndTimestamp<V> valueAndTimestamp); // returns `null` if `valueAndTimestamp==null`
> }

I don't think there is a need to revote. I updated the KIP accordingly.
Please let me know if there are any concern about this minor change.


-Matthias


On 4/19/19 1:27 AM, Matthias J. Sax wrote:
> Quick update to the KIP. While working on
> 
> https://github.com/apache/kafka/pull/6601
> 
> I realized that I forgot to list the following three new
> methods that we need to add in addition:
> 
>>     public static KeyValueBytesStoreSupplier persistentTimestampedKeyValueStore(final String name);
>>
>>     public static WindowBytesStoreSupplier persistentTimestampedWindowStore(final String name,
>>                                                                             final Duration retentionPeriod,
>>                                                                             final Duration windowSize,
>>                                                                             final boolean retainDuplicates);
>>
>>     public static SessionBytesStoreSupplier persistentTimestampedSessionStore(final String name,
>>                                                                               final Duration retentionPeriod);
> 
> I updated the KIP accordingly.
> 
> 
> I don't think there is any need to revote, because this is a minor and
> straight forward change to the KIP.
> 
> 
> -Matthias
> 
> 
> On 1/28/19 6:32 PM, Matthias J. Sax wrote:
>> Hi,
>>
>> during PR reviews, we discovered a couple of opportunities to simply and
>> improve the KIP and code. Thus, the following minor changes to the
>> public API are done (the KIP is already updated). I revote is not
>> necessary as the changes are minor.
>>
>>  - interface `ValueAndTimestamp` is going to be a class
>>
>>  - interface `RecordConverter` is renamed to `TimestampedBytesStore` and
>> we add a static method that converts values from old to new format
>>
>>  - the three new interfaces `TimestampedXxxStore` don't add any new methods
>>
>>
>>
>> Let us know if there are any objections. I can also provide more details
>> why those changes make sense.
>>
>> Thanks a lot!
>>
>>
>> -Matthias
>>
>>
>> On 1/18/19 10:00 PM, Matthias J. Sax wrote:
>>> +1 from myself.
>>>
>>>
>>> I am also closing this vote. The KIP is accepted with
>>>
>>> - 3 binding votes (Damian, Guozhang, Matthias)
>>> - 3 non-binding votes (Bill, Patrik, John)
>>>
>>>
>>> Thanks for the discussion and voting.
>>>
>>>
>>> -Matthias
>>>
>>>
>>> On 1/16/19 10:35 AM, John Roesler wrote:
>>>> +1 (nonbinding) from me.
>>>>
>>>> Thanks for the KIP, Matthias.
>>>>
>>>> -John
>>>>
>>>> On Wed, Jan 16, 2019 at 12:01 PM Guozhang Wang <wa...@gmail.com> wrote:
>>>>
>>>>> Thanks Matthias, I left some minor comments but since they do not involve
>>>>> in any major architectural changes and I did not feel strong about the
>>>>> naming etc as well. I'd +1 on the proposal as well.
>>>>>
>>>>> Feel free to reply / accept or reject my suggestions on the other DISCUSS
>>>>> thread.
>>>>>
>>>>>
>>>>> Guozhang
>>>>>
>>>>> On Wed, Jan 16, 2019 at 6:38 AM Damian Guy <da...@gmail.com> wrote:
>>>>>
>>>>>> +1
>>>>>>
>>>>>> On Wed, 16 Jan 2019 at 05:09, Patrik Kleindl <pk...@gmail.com> wrote:
>>>>>>
>>>>>>> +1 (non-binding)
>>>>>>> Thanks too
>>>>>>> Best regards
>>>>>>> Patrik
>>>>>>>
>>>>>>>> Am 16.01.2019 um 03:30 schrieb Bill Bejeck <bb...@gmail.com>:
>>>>>>>>
>>>>>>>> Thanks for the KIP Matthias.
>>>>>>>>
>>>>>>>> +1
>>>>>>>>
>>>>>>>> -Bill
>>>>>>>>
>>>>>>>> On Tue, Jan 15, 2019 at 7:33 PM Matthias J. Sax <
>>>>> matthias@confluent.io
>>>>>>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I would like to start the vote for KIP-258:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-258%3A+Allow+to+Store+Record+Timestamps+in+RocksDB
>>>>>>>>>
>>>>>>>>> The KIP adds new stores that allow to store record timestamps next
>>>>> to
>>>>>>>>> key and value. Additionally, we will allow to upgrade exiting stores
>>>>>> to
>>>>>>>>> the new stores; this will allow us to use the new stores in the DSL
>>>>>> with
>>>>>>>>> a smooth upgrade path.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -Matthias
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> -- Guozhang
>>>>>
>>>>
>>>
>>
> 
> 
>