Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i1824 guava cache wraps OOM error which is then considered NonFatal and leads to validation error #1825

Merged
merged 6 commits into from
Sep 13, 2022

Conversation

pragmaxim
Copy link
Collaborator

@pragmaxim pragmaxim commented Aug 31, 2022

Closes #1824

The problem is that Guava cache org.ergoplatform.nodeView.history.storage.HistoryStorage#headersCache and friends wrap OOM Error into unchecked ExecutionError like thiscom.google.common.util.concurrent.ExecutionError: java.lang.OutOfMemoryError: Java heap space and it is then considered NonFatal => if it happens during modifier processing it can trigger Validation Error and modifiers get invalidated 🤦

@pragmaxim pragmaxim marked this pull request as ready for review September 1, 2022 06:19
@pragmaxim pragmaxim changed the title i1824 prevent oom from invalidating block i1824 guava cache wraps OOM error which is then considered NonFatal and leads to validation error Sep 1, 2022
Base automatically changed from v4.0.42 to master September 1, 2022 20:09
.build[String, BlockSection]
private val blockSectionsCache =
Caffeine.newBuilder()
.maximumSize(config.history.headersCacheSize)
Copy link
Member

Choose a reason for hiding this comment

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

why headersCacheSize here ? Looks like a mistake.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Uff, I misread the lines

.build[ByteArrayWrapper, Array[Byte]]
private val indexCache =
Caffeine.newBuilder()
.maximumSize(config.history.headersCacheSize)
Copy link
Member

Choose a reason for hiding this comment

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

same, why headersCacheSize ?

@kushti kushti changed the base branch from master to v4.0.43 September 13, 2022 09:33
@kushti kushti merged commit 90f5d9b into v4.0.43 Sep 13, 2022
@kushti kushti deleted the i1824-prevent-oom-from-invalidating-blocks branch September 13, 2022 09:39
@kushti kushti mentioned this pull request Sep 16, 2022
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