I have performance concerns about this proposal, because each time a
window/session store is accessed, a new (unnecessary) object needs to be
created, and accessing the store in the processors is on the hot code path.
-Matthias
On 6/20/19 10:57 AM, John Roesler wrote:
> Hi again, all,
>
> Afte
Hi again, all,
After wrestling with some other issues around the window interface,
I'd propose that we consider normalizing the WindowStore (and
SessionStore) interfaces with respect to KeyValueStore. We can't
actually make the classes related because they'll clash on the
deprecated methods, but w
Let's see what other think about (2)
(3) Interesting. My reasoning follows the code:
For example, `RocksDbKeyValueBytesStoreSupplier#metricsScope()`returns
"rocksdb-state" and this is concatenated later in `MeteredKeyValueStore`
that adds a tag
key: metricScope + "-id"
value: nam
Hi Matthias,
2)
Yes, this is how I meant it.
I am still in favour of `value-by-...` instead of `get`, because it
gives you immediately the meaning of the metric without the need to
know the API of the stores.
3)
I asked to avoid misunderstandings because in KIP-444, `-state-` is
stated explicitly
Thanks Bruno.
I updated the KIP without changing the metric name yet -- want to
discuss this first.
1) I am also ok to keep `all()`
2) That's a good idea. To make sure I understand your suggestion
correctly, below the mapping between metric name an methods.
(Some metric names are use by multi
Hi all,
My comments on this KIP:
1. I would use `all()` instead of `range()` because the functionality
is immediately clear without the need to look at the parameter list.
2. I would decouple method names from metrics name, because this
allows us to change one naming independently from the other
Thanks John and Guozhang.
I also lean towards different metric names.
While updating the KIP, I also realized that the whole store API for
window and session store is actually rather inconsistent. Hence, I
extended the scope of this KIP to cleanup both interfaces and changed
the KIP name to
"
Thanks John.
I think I'm convinced about not collapsing the metrics measuring for
various function calls. Regarding the possible solution I'm a bit inclined
to just distinguish on the metric names directly over on tags: since our
current metrics naming are a tad messed up anyways, fixing them in o
Thanks all (or range? ;) ) for the discussion. Good points all around.
Although I see the allure of naming the metrics the same as the things
they're measuring, it seems not to be perfect. Seconding Matthias's latter
thought, I think it's likely you'd want to measure the method calls
independently
Thanks for the input Guozhang. I was not aware of those dependencies.
It might be good to align this KIP with the metrics cleanup. Not sure
atm, if we should use different metric names for different overloads,
even if those have the same method name?
If we rename all method to `range()` and use t
While working at KIP-444 (https://github.com/apache/kafka/pull/6498) I
realized there are a bunch of issues on metric names v.s. function names,
e.g. some function named `fetchAll` are actually measure with `fetch`, etc.
So in that KIP I proposed to make the function name aligned with metrics
name.
I did not see a reason to rename `all()` to `range()`. `all()` does not
take any parameters to limit a range and is a good name IMHO. But I am
not married to keep `all()` and if we think we should rename it, too, I
am fine with it.
Not sure what connection you make to metrics though. Can you elabo
I like the renaming, since it also aligns with our metrics cleanup
(KIP-444) which touches upon the store level metrics as well.
One question: you seems still suggesting to keep "all" with the current
name (and also using a separate metric for it), what's the difference
between this one and other
Thanks for the input.
>> Just to clarify the naming conflicts is between the newly added function
>> and the old functions that we want to deprecate / remove right? The
Yes, the conflict is just fort the existing `fetch()` methods for which
we want to change the return type.
IMHO, we should not
Hello Matthias,
Just to clarify the naming conflicts is between the newly added function
and the old functions that we want to deprecate / remove right? The
existing ones have different signatures with parameters so that they should
not have conflicts.
I was thinking about just make the change di
I am open to change the return type to
KeyValueIterator, V>
However, this requires to rename
#fetch(K key, long startTimestamp, long endTimestamp)
#fetch(K key, Instant startTimestamp, Instant endTimestamp)
to avoid naming conflicts.
What new name would you suggest? The existing methods are ca
I was thinking about changing the return type even, to
`KeyValueIterator, V>` since it is confusing to users about the
key typed `Long` (Streams javadoc today did not explain it clearly either),
note it is not backward compatible at all.
Personally I'd prefer to just deprecate the API and new new
I remember thinking this while working on window stores, am definitely for
it.
On Mon, Mar 11, 2019 at 9:20 AM John Roesler wrote:
> Sounds great to me. Thanks, Matthias!
> -John
>
> On Sun, Mar 10, 2019 at 11:58 PM Matthias J. Sax
> wrote:
>
> > Hi,
> >
> > I would like to propose KIP-439 to d
Sounds great to me. Thanks, Matthias!
-John
On Sun, Mar 10, 2019 at 11:58 PM Matthias J. Sax
wrote:
> Hi,
>
> I would like to propose KIP-439 to deprecate interface
> `WindowStoreIterator`.
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-439%3A+Deprecate+Interface+WindowStoreIterator
Hi,
I would like to propose KIP-439 to deprecate interface
`WindowStoreIterator`.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-439%3A+Deprecate+Interface+WindowStoreIterator
Looking forward to your feedback.
-Matthias
signature.asc
Description: OpenPGP digital signature
20 matches
Mail list logo
