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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate original callstack, save a fiber allocation if unnecessary #503

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vici37
Copy link

@Vici37 Vici37 commented Jan 21, 2024

I'm working on a project that's generating a couple million rows to enter into a sqlite database. For reasons I haven't figured out yet, I'm getting a "cannot allocate memory" error when a new spawn call is being made (I've verified the machine this program is running on has plenty of memory and isn't getting anywhere near the limit, and other fiber allocations seem to be working just fine 馃し ). Regardless of the what's causing that, I was able to update some error propagation in granite to discover an unnecessary spawn call being made in the event the @@reader_adapter and @@writer_adapter are the same adapter. Making this PR to include the error propagation changes as well as fixing the unnecessary spawn call.

@@ -68,7 +68,7 @@ module Granite::Transactions
end
{% end %}
rescue err
raise DB::Error.new(err.message)
raise DB::Error.new(err.message, cause: err)
Copy link
Author

Choose a reason for hiding this comment

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

These were originally swallowing the underlying callstacks and only propagating the cannot allocate memory error message. Adding cause let me find where the root of the problem is (not the why, though, hrm).

@@ -51,6 +51,8 @@ module Granite::ConnectionManagement
end

def self.schedule_adapter_switch
return if @@writer_adapter == @@reader_adapter
Copy link
Author

Choose a reason for hiding this comment

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

Not making a spawn call here saves ~8mb of memory allocation due to a fiber stack allocation, and saves me from my problem (and I'm hoping saves others from unnecessary memory allocation as well).

@Vici37
Copy link
Author

Vici37 commented Jan 21, 2024

An additional thought: I've been running into this cannot allocate memory issue when using Granite's import method with largish batches (~1k - 10k objects each batch). If that spawn was running for each of model in the after_save block, then would that have allocated an additional ~1k-10k x fibers and memory for their stacks. The documentation for fiber stack memory is that it has "stack size of 8MiB", but that "only 4kib are actually allocated".

I'm wondering if during the import method call, that adapter switching couldn't be made more efficient as to not create more fibers than needed.

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.

None yet

1 participant