[GitHub] [kafka] szpak opened a new pull request #10062: MINOR: Add performAndClose default method in KeyValueIterator

[GitBox <gi...@apache.org>]

szpak opened a new pull request #10062:
URL: https://github.com/apache/kafka/pull/10062


   That method intends to increase a chance to have KeyValueIterator closed
   after usage, by providing a convenient performAndClose() default method which
   executes a given operation and guarantee to automatically close the iterator
   right after.
   
   ### Rationality
   
   I decided to create that PR observing in different projects how often an iterator is left open after performing operations such as 
   `.forEachRemaining(...)` (who reads JavaDoc after all? ;-) ). For people aware of the problem, instead of verbose try-with-resources constructions repeated in every place in code:
   
   ```
   try (KeyValueIterator<String, Order> ordersKeyValueIterator =
            streamsBuilderFactoryBean
                .getKafkaStreams()
                .store(StoreQueryParameters.fromNameAndType(ORDERS_STORE,
                    QueryableStoreTypes.<String, Order>keyValueStore()))
                .all()) {
       ordersKeyValueIterator
           .forEachRemaining(kv -> {
               ...
           });
   }
   ```
   (or hidden in in-house built utility classes)
   
   a developer using Kafka Streams has a built-in clear way to perform operations on elements contained in an iterator (and have that stream closed after that automatically):
   ```
   streamsBuilderFactoryBean
       .getKafkaStreams()
       .store(StoreQueryParameters.fromNameAndType(ORDERS_STORE,
           QueryableStoreTypes.<String, OrderRequest>keyValueStore()))
       .all()
       .performAndClose(iterator -> iterator
           .forEachRemaining(kv -> {
               ...
           })
       );
   ```
   
   I believe it can increase a rate of closed iterator in user projects.
   
   ### Testing approach
   
   Being a default method in KeyValueIterator, I decided to test it using KeyValueIteratorFacade which already provides a nice unit testing infrastructure with mocked iterator. I don't know the implementation details, but I expect to have it behaved similarly in other implementations.
   I might miss some extra cases, so feel free to point them and I will happily cover them with tests.
   
   ### Rejected extensions
   
   For some time past, I was thinking also about covering operations that returns some value. However, at least in projects I've been observing the usage of Kafka Streams, performing side effects is more popular and implementation for that use cases might be more tricky and I decided to start with pure Consumer.
   
   ### Other information
   
   Naming is not my area of expertise. Feel free to propose any better name or any other possible enhancements to my MR :-).
   
   ### 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] chia7712 commented on pull request #10062: MINOR: Add performAndClose default method in KeyValueIterator

[GitBox <gi...@apache.org>]

chia7712 commented on pull request #10062:
URL: https://github.com/apache/kafka/pull/10062#issuecomment-795146375


   @szpak thanks for this contribution.
   
   I'm not familiar with `KeyValueIterator` but it appears to be a public APIs changes. If so, a KIP is required. see https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] szpak commented on pull request #10062: MINOR: Add performAndClose default method in KeyValueIterator

[GitBox <gi...@apache.org>]

szpak commented on pull request #10062:
URL: https://github.com/apache/kafka/pull/10062#issuecomment-795123807


   Anyone willing to review my PR?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org