Matthew-DWM commented on code in PR #9008:
URL: https://github.com/apache/texera/pull/9008#discussion_r4298664581


##########
amber/src/main/python/core/models/internal_queue.py:
##########
@@ -55,13 +56,24 @@ class ECMElement(InternalQueueElement):
 T = TypeVar("T", bound=InternalQueueElement)
 
 
+def _in_mem_size(item) -> int:
+    """
+    Bytes an item holds while queued. The total is reported to the JVM as
+    queued credit for flow control, so a data frame counts its Arrow buffers
+    rather than the few bytes of its Python wrapper.
+    """
+    if isinstance(item, DataElement) and isinstance(item.payload, DataFrame):
+        return item.payload.frame.get_total_buffer_size()
+    return sys.getsizeof(item)

Review Comment:
   Good point! In the code, there doesn't look like to be an enforced upper 
bound on a `State`, holding whatever the operator puts in it (likely loop 
variables being ~100B of JSON). One difference between this and a `DataFrame` 
are that states only come from the state hooks and their number is fixed by the 
workflow's shape, not data volume. One caveat is that each downstream worker 
gets a copy from every upstream worker. Also, the JVM side is currently 
charging every StateFrame a flat 200B (WorkflowMessage.getInMemSize).
   
   If large states are realistic and you recommend sizing `StateFrame`, I can 
address in a follow-up (new issue/PR or add to this?). Could be to size a 
`StateFrame` by the Arrow table it arrives in (~0.4 µs per state) and cap the 
JVM-side charge at the credit limit.



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