Skip to content

Change ManifestList to an immutable list from linked list#1814

Closed
johnclara wants to merge 1 commit intoapache:masterfrom
johnclara:manifestArrayList
Closed

Change ManifestList to an immutable list from linked list#1814
johnclara wants to merge 1 commit intoapache:masterfrom
johnclara:manifestArrayList

Conversation

@johnclara
Copy link

@johnclara johnclara commented Nov 24, 2020

From #1812

Trying to match with: https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/BaseSnapshot.java#L131

Not sure if this is worth the risk of changing. A couple of our tables have some really large manifest lists and I was wondering if the micro benchmark shows anything noticable

.commit();

List<ManifestFile> manifests = table.currentSnapshot().allManifests();
List<ManifestFile> manifests = new ArrayList(table.currentSnapshot().allManifests());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Author

@johnclara johnclara Nov 24, 2020

Choose a reason for hiding this comment

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

@rdblue sorting isn't supported on immutable lists.

Sorry, I should've closed this earlier. I was thinking this morning that this definitely isn't worth the risk of changing

Sorry for the at

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem!

@johnclara
Copy link
Author

not worth changing

@johnclara johnclara closed this Nov 24, 2020
@johnclara johnclara deleted the manifestArrayList branch November 24, 2020 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments