Skip to content

THREESCALE-12061: Monkey patch ruby-oci8 to fix memory bloat (master) #4176

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.

Sign up for GitHub

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

Closed
mayorova wants to merge 1 commit into from
Closed

THREESCALE-12061: Monkey patch ruby-oci8 to fix memory bloat (master) #4176

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions config/initializers/oracle.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# frozen_string_literal: true

# Disable array fetching in OCI8 to avoid memory bloat
# This must be patched before any database connections are made
# See: https://github.com/kubo/ruby-oci8/issues/230
if Gem.loaded_specs.key?('ruby-oci8')
require "oci8"

module OCI8CursorMemoryPatch
private

def define_one_column(pos, param)
@fetch_array_size = nil # disable array fetching anytime
super # call original
end
end

OCI8::Cursor.prepend(OCI8CursorMemoryPatch)
Comment on lines +9 to +18
Copy link
Contributor

@akostadinov akostadinov Nov 24, 2025

Choose a reason for hiding this comment

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

I think we can just move this down within the if System::Database.oracle? block and avoid the custom oci check.

Copy link
Contributor Author

@mayorova mayorova Nov 24, 2025

Choose a reason for hiding this comment

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

Well, that's what I thought first. But there is an issue. That .oracle? check requires ActiveRecord::DatabaseConfigurations to be loaded, and I think loading ActiveRecord first prevents the patch from being applied correctly (I guess because it might be pulling ruby-oci8 as a depencency).

So, I decided to opt for a safer check, that doesn't require ActiveRecord to be loaded.

Copy link
Contributor

@akostadinov akostadinov Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think that being already loaded is not a problem. It is not something that we read at startup and then stays unchanged. It is just changing the runtime method behavior and the time it happens is not relevant.

Copy link
Contributor Author

@mayorova mayorova Nov 24, 2025

Choose a reason for hiding this comment

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

I'm not sure... I just know that when the patch was inside the block

ActiveSupport.on_load(:active_record) do
  if System::Database.oracle?

the QE tests were still showing the regression.
But I was also using another approach (not a module prepend that you suggested), in case it might have any effect.

end

ActiveSupport.on_load(:active_record) do
if System::Database.oracle?
require 'arel/visitors/oracle12_hack'
Expand Down