Skip to content

perf issues in google-cloud-bigquery-storage - #1816

Open
pjfanning wants to merge 3 commits into
apache:mainfrom
pjfanning:bigq
Open

perf issues in google-cloud-bigquery-storage#1816
pjfanning wants to merge 3 commits into
apache:mainfrom
pjfanning:bigq

Conversation

@pjfanning

@pjfanning pjfanning commented Aug 4, 2026

Copy link
Copy Markdown
Member

P-06 (Arrow allocator):

  • SimpleRowReader now takes allocatorBytes parameter (no default)
  • BigQueryStorageSettings gains arrowAllocatorBytes field (default 512MB)
  • Configurable via withArrowAllocatorBytes() or config key arrowAllocatorBytes (supports size suffixes like 256 MB)
  • Threaded through ArrowSource → BigQueryArrowStorage → GrpcBigQueryStorageReader

P-07 (readMerged):

  • Seq.reduce((a, b) => a.merge(b)) kept — it's Seq.reduce on Source.merge which is streaming, not buffering
  • Fixed SimpleRowReader leak in readRecordsMerged — now wrapped in try/finally reader.close()

read(client, session)
.map { a =>
a.map(new SimpleRowReader(session.schema.arrowSchema.get).read(_))
a.map(new SimpleRowReader(session.schema.arrowSchema.get, allocatorBytes).read(_))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SimpleRowReader created here is never closed — each batch allocates a fresh RootAllocator that is never released, leaking native memory over time.

readRecordsMerged now correctly wraps the reader in try/finally reader.close(), but this path does not. Consider mirroring the same pattern:

a.map { batch =>
  val reader = new SimpleRowReader(session.schema.arrowSchema.get, allocatorBytes)
  try reader.read(batch)
  finally reader.close()
}.mapConcat(c => c)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed this based on your feedback, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants