You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "jinyongchoi (via GitHub)" <gi...@apache.org> on 2024/03/08 05:23:25 UTC
[PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
jinyongchoi opened a new pull request, #15495:
URL: https://github.com/apache/kafka/pull/15495
Related Issue
[KAFKA-15302](https://issues.apache.org/jira/browse/KAFKA-15302)
This PR adds documentation regarding the potential return of stale values when using 'withCachingEnabled' and deleting keys during store iteration.
Since there's a current solution involving flush(), modifications may not be made immediately. Hence, it's necessary to explicitly state in the documentation.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #15495:
URL: https://github.com/apache/kafka/pull/15495#issuecomment-2177315273
Thanks for the PR @jinyongchoi -- and sorry for the delay -- seems the PR slipped through the cracks...
Merged to `trunk` and cherry-picked to `3.8` branch (just in time for 3.8 release :))
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "jinyongchoi (via GitHub)" <gi...@apache.org>.
jinyongchoi commented on code in PR #15495:
URL: https://github.com/apache/kafka/pull/15495#discussion_r1527410280
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
+ the value of the key that hasn't been flushed from the cache may be returned as a stale value.
+ Therefore, when deleting, you must <code class="docutils literal"><span class="pre">flush()</span></code> the cache before the iterator.</p>
Review Comment:
> I would rephrase this sentenc.
> ```
> To avoid reading stale data, you can `flush()` the store before creating the iterator. Note, that flushing too often can lead to performance degration if RocksDB is used, so we advice to avoid flushing manually in general.
Ok. I will change.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "jinyongchoi (via GitHub)" <gi...@apache.org>.
jinyongchoi commented on code in PR #15495:
URL: https://github.com/apache/kafka/pull/15495#discussion_r1527474042
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
+ the value of the key that hasn't been flushed from the cache may be returned as a stale value.
+ Therefore, when deleting, you must <code class="docutils literal"><span class="pre">flush()</span></code> the cache before the iterator.</p>
Review Comment:
@mjsax Could you verify the changes I fixed?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "jinyongchoi (via GitHub)" <gi...@apache.org>.
jinyongchoi commented on code in PR #15495:
URL: https://github.com/apache/kafka/pull/15495#discussion_r1527393749
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
Review Comment:
> Cannot remember from the ticket, but was this limited to deletes, or also updates?
Yes, that's correct.
This affects the snapshot created in store.all(), causing it to read the values from rocksdb before the cache is flushed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "jinyongchoi (via GitHub)" <gi...@apache.org>.
jinyongchoi commented on code in PR #15495:
URL: https://github.com/apache/kafka/pull/15495#discussion_r1527474042
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
+ the value of the key that hasn't been flushed from the cache may be returned as a stale value.
+ Therefore, when deleting, you must <code class="docutils literal"><span class="pre">flush()</span></code> the cache before the iterator.</p>
Review Comment:
Could you verify the changes I fixed?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax merged PR #15495:
URL: https://github.com/apache/kafka/pull/15495
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15495:
URL: https://github.com/apache/kafka/pull/15495#issuecomment-2171031137
This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch) <p> If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-15302: Stale value returned when using store.all() with key deletion [docs] [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #15495:
URL: https://github.com/apache/kafka/pull/15495#discussion_r1524038413
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
+ the value of the key that hasn't been flushed from the cache may be returned as a stale value.
+ Therefore, when deleting, you must <code class="docutils literal"><span class="pre">flush()</span></code> the cache before the iterator.</p>
Review Comment:
I would rephrase this sentenc.
```
To avoid reading stale data, you can `flush()` the store before creating the iterator. Note, that flushing too often can lead to performance degration if RocksDB is used, so we advice to avoid flushing manually in general.
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
Review Comment:
Cannot remember from the ticket, but was this limited to deletes, or also updates?
##########
docs/streams/developer-guide/memory-mgmt.html:
##########
@@ -151,6 +151,10 @@
Serdes.Long())
.withCachingEnabled();</code></pre>
<p>Record caches are not supported for <a class="reference internal" href="processor-api.html#streams-developer-guide-state-store-versioned"><span class="std std-ref">versioned state stores</span></a>.</p>
+ <p><strong>Caution:</strong> When using <code class="docutils literal"><span class="pre">withCachingEnabled()</span></code>,
+ if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the Store while iterating(e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
Review Comment:
```suggestion
if you <code class="docutils literal"><span class="pre">delete()</span></code> a key from the store while iterating (e.g., <code class="docutils literal"><span class="pre">KeyValueIterator</span></code>) through the keys,
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org