Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Export SpanData in batches of maximum bufferSize.#1882

Merged
bogdandrutu merged 2 commits intocensus-instrumentation:masterfrom
bogdandrutu:batch
May 7, 2019
Merged

Export SpanData in batches of maximum bufferSize.#1882
bogdandrutu merged 2 commits intocensus-instrumentation:masterfrom
bogdandrutu:batch

Conversation

@bogdandrutu
Copy link
Copy Markdown
Contributor

No description provided.

@bogdandrutu bogdandrutu requested review from a team, dinooliva, rghetia and songy23 as code owners May 6, 2019 23:01

final List<SpanData> spanDataList = fromSpanImplToSpanData(spansCopy);
private void exportBatches(ArrayList<RecordEventsSpanImpl> spanList) {
ArrayList<SpanData> spanDataList = new ArrayList<>(bufferSize);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just use Lists.partition?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Never mind, forgot we don't want to depend on Guava here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not only that, but we want to remove the reference to the RecordEventsSpanImpl asap, with Lists.partitions they use List.subList which is implemented as a wrapper on top of the original list also immutable so it is harder to remove the reference to that object.

Copy link
Copy Markdown
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM, please fix the nullness checker.

}

final List<SpanData> spanDataList = fromSpanImplToSpanData(spansCopy);
private void exportBatches(ArrayList<RecordEventsSpanImpl> spanList) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be worth noting that spanList is assumed not to be shared across other threads.

@bogdandrutu bogdandrutu merged commit 54e4bba into census-instrumentation:master May 7, 2019
@bogdandrutu bogdandrutu deleted the batch branch May 7, 2019 17:32
@bogdandrutu bogdandrutu mentioned this pull request May 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants