-
Notifications
You must be signed in to change notification settings - Fork 72
Oracle perf #4187
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
base: master
Are you sure you want to change the base?
Oracle perf #4187
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| ActiveSupport.on_load(:active_record) do | ||
| if System::Database.oracle? | ||
| require 'arel/visitors/oracle12_hack' | ||
| require 'arel/visitors/oracle12_hack' || next # once done, we can skip setup | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe on application reload we don't need any of this code to be executed again. Using this |
||
|
|
||
| # in 6.0.6 automatic detection of max identifier length was introduced | ||
| # see https://github.com/rsim/oracle-enhanced/pull/1703 | ||
|
|
@@ -30,9 +30,82 @@ def column(name, type, **options) | |
| end | ||
| end) | ||
|
|
||
| ENV['NLS_LANG'] ||= 'AMERICAN_AMERICA.UTF8' | ||
| # clean-up prepared statements/cursors on connection return to pool | ||
| module OracleStatementCleanup | ||
| def self.included(base) | ||
| base.set_callback :checkin, :after, :close_and_clear_statements | ||
| end | ||
|
|
||
| def close_and_clear_statements | ||
| start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC) | ||
| statement_count = @statements&.length || 0 | ||
| @statements&.clear | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some AI explanation why clearing cursors from app end may not be much of a performance penalty because oracle server still caches these for reuse.
|
||
|
|
||
| # Query V$OPEN_CURSOR to see current cursor count for this session | ||
| # Count application cursors (OPEN for regular SQL, PL/SQL for stored procs/funcs) | ||
| # Exclude Oracle's internal cursors (DICTIONARY LOOKUP, OPEN RECURSIVE, etc.) | ||
| begin | ||
| cursor_count = select_value(<<~SQL) | ||
| SELECT COUNT(*) | ||
| FROM V$OPEN_CURSOR | ||
| WHERE SID = SYS_CONTEXT('USERENV', 'SID') | ||
| AND CURSOR_TYPE IN ('OPEN', 'PL/SQL') | ||
| AND (SQL_TEXT IS NULL OR SQL_TEXT NOT LIKE '%V$OPEN_CURSOR%') | ||
| SQL | ||
| duration = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time | ||
| Rails.logger.info "#{statement_count} statements cleared from AR pool on checkin in #{duration.round(3)}s (Oracle reports #{cursor_count} cursors in session)" | ||
| rescue => e | ||
| Rails.logger.warn "Failed to query V$OPEN_CURSOR: #{e.message}" | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ActiveRecord::Base.skip_callback(:update, :after, :enhanced_write_lobs) | ||
|
|
||
| # Supposedly in the past `update_all` and `where` used this with issues. | ||
| # https://github.com/rsim/oracle-enhanced/issues/1588#issue-272289146 | ||
| # The default behaviour is to serialize them to 'empty_clob()' basically wiping out the data. | ||
| # The team behind it believes `Table.update_all(column: 'text')` | ||
| # should wipe all your data in that column: https://github.com/rsim/oracle-enhanced/issues/1588#issuecomment-343353756 | ||
| # Might be dead code now. | ||
jlledom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| module OracleEnhancedSmartQuoting | ||
| CLOB_INLINE_LIMIT = 32767 # 32KB - 1 | ||
| BLOB_INLINE_LIMIT = 16383 # 16KB - 1 (hex encoding doubles size) | ||
|
|
||
| def quote(value) | ||
| case value | ||
| when ActiveModel::Type::Binary::Data | ||
| raw = value.to_s | ||
| size = raw.bytesize | ||
|
|
||
| if size == 0 | ||
| "empty_blob()" | ||
| elsif size <= BLOB_INLINE_LIMIT | ||
| "hextoraw('#{raw.unpack1('H*')}')" | ||
| else | ||
| raise ArgumentError, "BLOB too large for inline quoting (#{size} bytes, max #{BLOB_INLINE_LIMIT} bytes). Use bind parameters instead." | ||
jlledom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| end | ||
| when ActiveRecord::Type::OracleEnhanced::Text::Data | ||
| text = value.to_s | ||
| size = text.bytesize | ||
|
|
||
| if size == 0 | ||
| "empty_clob()" | ||
| elsif size <= CLOB_INLINE_LIMIT | ||
| "to_clob(#{super(text)})" | ||
| else | ||
| raise ArgumentError, "CLOB too large for inline quoting (#{size} bytes, max #{CLOB_INLINE_LIMIT} bytes). Use bind parameters instead." | ||
| end | ||
| else | ||
| super | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.class_eval do | ||
| include OracleStatementCleanup | ||
| prepend OracleEnhancedSmartQuoting | ||
|
|
||
| # Fixing OCIError: ORA-01741: illegal zero-length identifier | ||
| # because of https://github.com/rails/rails/commit/c18a95e38e9860953236aed94c1bfb877fa3be84 | ||
| # the value of `columns` is [ "\"ACCOUNTS\".\"ID\"" ] which forms an incorrect query | ||
|
|
@@ -65,24 +138,6 @@ def add_column(table_name, column_name, type, **options) | |
| end | ||
| end | ||
|
|
||
| # We need to patch Oracle Adapter quoting to actually serialize CLOB columns. | ||
| # https://github.com/rsim/oracle-enhanced/issues/1588#issue-272289146 | ||
| # The default behaviour is to serialize them to 'empty_clob()' basically wiping out the data. | ||
| # The team behind it believes `Table.update_all(column: 'text')` | ||
| # should wipe all your data in that column: https://github.com/rsim/oracle-enhanced/issues/1588#issuecomment-343353756 | ||
| # So we try to convert the text to using `to_clob` function. | ||
| def _quote(value) | ||
| case value | ||
| when ActiveModel::Type::Binary::Data | ||
| # I know this looks ugly, but that just modified copy paste of what the adapter does (minus the rescue). | ||
| # It is a bit improved in next version due to ActiveRecord Attributes API. | ||
| %{to_blob(#{quote(value.to_s)})} | ||
| when ActiveRecord::Type::OracleEnhanced::Text::Data | ||
| %{to_clob(#{quote(value.to_s)})} | ||
| else | ||
| super | ||
| end | ||
| end | ||
| end) | ||
| end | ||
|
|
||
|
|
@@ -273,5 +328,23 @@ def column_definitions(table_name) | |
| end | ||
|
|
||
| ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.prepend OracleEnhancedAdapterSchemaIssue2276 | ||
|
|
||
| # see https://github.com/kubo/ruby-oci8/pull/271 | ||
| module OCI8DisableArrayFetch | ||
| private | ||
| def define_one_column(pos, param) | ||
| @fetch_array_size = nil # disable memory array fetching anytime | ||
| super # call original | ||
| end | ||
jlledom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| end | ||
|
|
||
| OCI8::Cursor.prepend(OCI8DisableArrayFetch) | ||
|
|
||
| # see https://github.com/kubo/ruby-oci8/pull/271 | ||
| # Enable piecewise retrieval for both CLOBs and BLOBs | ||
| # With the OCIConnectionCursorLobFix above, we can safely use both mappings | ||
| # because LOBs are bound as OCI8::CLOB/BLOB objects, not LONG data | ||
| OCI8::BindType::Mapping[:clob] = OCI8::BindType::Long | ||
| OCI8::BindType::Mapping[:blob] = OCI8::BindType::LongRaw | ||
jlledom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| end | ||
|
|
||
| After do | ||
| ActiveRecord::Base.clear_active_connections! | ||
| ActiveRecord::Base.connection_handler.clear_active_connections! | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoid DEPRECATION warning |
||
| end | ||
|
|
||
| Around '@security' do |scenario, block| | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'test_helper' | ||
|
|
||
| class OracleLobLargeUpdateTest < ActiveSupport::TestCase | ||
| # Test large LOB (CLOB and BLOB) updates with OCI8::BindType::Long/LongRaw piecewise retrieval | ||
| # This verifies the OCIConnectionCursorLobFix in config/initializers/oracle.rb works correctly | ||
| # by using OCI8::CLOB.new() and OCI8::BLOB.new() for direct binding | ||
|
|
||
| setup do | ||
| skip "Only run on Oracle database" unless System::Database.oracle? | ||
|
|
||
| @service = FactoryBot.create(:simple_service) | ||
| @proxy = @service.proxy | ||
| end | ||
|
|
||
| test "update and retrieve large CLOB (policies_config) with 512KB data" do | ||
| # Generate ~512KB of JSON data for policies_config | ||
| large_policy_data = generate_large_policies_config(2.kilobytes) | ||
|
|
||
| # Initial save using ActiveRecord | ||
| @proxy.policies_config = large_policy_data | ||
| @proxy.save! | ||
|
|
||
| @proxy.reload | ||
| retrieved_policies = @proxy.policies_config | ||
| assert retrieved_policies.to_json.bytesize >= 2.kilobytes | ||
|
|
||
| expected_policy = JSON.parse(large_policy_data).first | ||
| assert_includes retrieved_policies.map(&:to_h), expected_policy | ||
|
|
||
| # NOW UPDATE with different data | ||
| large_policy_data_v2 = generate_large_policies_config(512.kilobytes, version: "2.0") | ||
| @proxy.reload | ||
| @proxy.policies_config = large_policy_data_v2 | ||
| @proxy.save! | ||
|
|
||
| @proxy.reload | ||
| retrieved_policies_v2 = @proxy.policies_config | ||
|
|
||
| assert retrieved_policies_v2.to_json.bytesize >= 512.kilobytes | ||
|
|
||
| expected_policy_v2 = JSON.parse(large_policy_data_v2).first | ||
| assert_includes retrieved_policies_v2.map(&:to_h), expected_policy_v2 | ||
| end | ||
|
|
||
| test "update and retrieve large BLOB (MemberPermission service_ids) with 512KB data" do | ||
| # Simple test model double for MemberPermission to avoid the JSON serialization logic | ||
| test_class = Class.new(ActiveRecord::Base) do | ||
| self.table_name = 'member_permissions' | ||
| end | ||
|
|
||
| small_binary_data = Random.bytes(2.kilobytes) | ||
| test_record = test_class.create!(service_ids: small_binary_data) | ||
| test_record.reload | ||
| assert_equal small_binary_data, test_record.service_ids | ||
|
|
||
| # NOW UPDATE with different random binary data | ||
| large_binary_data = Random.bytes(512.kilobytes) | ||
| test_record.service_ids = large_binary_data | ||
| test_record.save! | ||
|
|
||
| test_record.reload | ||
| retrieved_value = test_record.service_ids | ||
|
|
||
| assert_equal large_binary_data.bytesize, retrieved_value.bytesize,"Updated binary data size should match" | ||
| assert_equal large_binary_data, retrieved_value,"Updated service_ids should match new binary data" | ||
| end | ||
|
|
||
| # practical max is 2GB - 8 bytes; can be increased with a OCI8::BLOB and CLOB fixes to write big data in chunks | ||
| test "update CLOB with multiple sizes to verify across internal thresholds" do | ||
| sizes = [ | ||
| 3.kilobytes, # Smaller than Oracle VARCHAR2 limit | ||
| 31.kilobytes, # Larger than standard VARCHAR2, within EXTENDED limit | ||
| 256.kilobytes # Large size | ||
| ] | ||
|
|
||
| sizes.each do |size| | ||
| large_data = generate_large_policies_config(size) | ||
|
|
||
| # Update using ActiveRecord (this will trigger write_lobs) | ||
| @proxy.policies_config = large_data | ||
| @proxy.save! | ||
|
|
||
| actual_policy = @proxy.reload.policies_config | ||
| assert_includes actual_policy, OpenStruct.new(JSON.parse(large_data).first) | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # Generate large JSON policies config data | ||
| def generate_large_policies_config(target_size, version="1.0") | ||
| # Generate a test JSON and calculate actual overhead | ||
| test_policy = { | ||
| "name" => "test_policy", | ||
| "version" => version, | ||
| "configuration" => { "data" => "" }, | ||
| "enabled" => true | ||
| } | ||
|
|
||
| test_json = [test_policy].to_json | ||
| padding_size = target_size - test_json.bytesize | ||
|
|
||
| return test_json if padding_size <= 0 | ||
|
|
||
| test_policy["configuration"]["data"] = "X" * padding_size | ||
| [test_policy].to_json | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another attempt at setting reliably
NLS_LANGbefore oracle connection is established and only if using oracle. Might be a hack but I'm not sure any other place would be less of a hack and I'm tired of trying to figure out where that other place might be. So here I'm sure it's gonna be set prior loading the oracle gems.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we have encoding problems before this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can see always a complaint that we fallback to some 7bit ASCII encoding if you don't set this variable separately in your environment. But we have always tried to set a default in the initializer. But that has been too late. So this one should be early enough always.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can see that line removed from the initializer