PDGGK commented on PR #37530:
URL: https://github.com/apache/beam/pull/37530#issuecomment-3863669136

   Thanks @Eliaaazzz, this is a very good point!
   
   You're absolutely right that `ValueState<List>` can be preferable for:
   - **Small collections**: Single RPC round-trip, no iterator/paging overhead
   - **Atomic full-overwrite patterns**: Read-modify-write the entire list as a 
unit
   
   I'll make the following changes:
   
   1. **Downgrade to INFO**: Change `LOG.warn` to `LOG.info` - this is a 
performance hint, not a correctness issue
   2. **Refine message**: Make the trade-off explicit:
      - Add: "This is appropriate for small collections or atomic replacement"
      - Keep: "For large collections or frequent appends, consider using..."
   3. **Add runner caveat**: Note that specialized state types 
(MapState/SetState) may not be supported by all runners
   
   This way we guide new users without alarming experienced users who 
intentionally use this pattern for valid use cases.
   
   Thanks for the thorough review!


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to