Strikerrx01 commented on code in PR #34135:
URL: https://github.com/apache/beam/pull/34135#discussion_r1979579457
##########
sdks/python/apache_beam/io/gcp/bigquery_tools.py:
##########
@@ -351,6 +352,9 @@ class BigQueryWrapper(object):
TEMP_DATASET = 'beam_temp_dataset_'
HISTOGRAM_METRIC_LOGGER = MetricLogger()
+
+ # Default TTL for cached table definitions in seconds
+ DEFAULT_TABLE_DEFINITION_TTL = 3600 # 1 hour
Review Comment:
@sjvanrossum Thanks for pointing this out. You're right that 1 hour TTL is
too long for streaming pipelines. Here are my suggestions for improvement:
1. Change the default TTL to 1 second to match Java SDK's behavior:
DEFAULT_TABLE_DEFINITION_TTL = 1 # Match Java SDK's TTL for consistency
2. Use cachetools.TTLCache with thread-safe implementation:
from cachetools import TTLCache
from cachetools.keys import hashkey
from functools import partial
#Use thread-safe TTLCache with proper key function
self.table_cache = TTLCache(
maxsize=self.DEFAULT_CACHE_MAX_SIZE,
ttl=self.DEFAULT_TABLE_DEFINITION_TTL,
getsizeof=None)
3. Add documentation about TTL implications: TTL is kept short (1s) to
ensure table schema changes propagate quickly
in streaming pipelines while still providing protection against excessive
API calls in high-throughput scenarios
Would these changes align better with the expected behavior? I can also add
configuration options to allow users to tune the TTL if needed for specific use
cases.
--
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]
