reuvenlax commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796437016


   First comments
      - This change isn't likely to improve any performance. Temp tables are 
only generated if the BQ load job is loading more than 11 terabytes of data, 
otherwise the data is simply loaded directly into BQ from files. This means 
that the vast majority of BQ loads never generate temp tables, and even if a 
single job was loading 100TB, it would only generate 10 temp table. 
      - For streaming, this change will break update compatibility for the 
Dataflow runner. We usually try to avoid this, and the minor savings here 
probably doesn't justify this. That being said, there's another PR - pr/12968 - 
that fixes a critical bug that might also break update compatibility, so if we 
can't figure out how to fix that bug in a compatible way, we might have a 
window to make changes.
     
   Looking at pr/12968, I think it actually misses fixing the pane-index bug 
for the WriteRename path, and your change here is a step towards fixing that 
(though the actual fix I think is a bit more complicated than what you have 
here). I suggest that we merge these two PRs.


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


Reply via email to