SQLite: Allow configurable sqlite database pragma variables. #5135

dangell7 · 2024-09-15T11:21:50Z

High Level Overview of Change

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

Public API: New feature (new methods and/or new fields)
Public API: Breaking change (in general, breaking changes should only impact the next api_version)
libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
Peer protocol change (must be backward compatible or bump the peer protocol version)

codecov · 2024-09-15T11:43:44Z

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.2%. Comparing base (0ece395) to head (dadc84a).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/app/rdb/detail/Vacuum.cpp	0.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #5135   +/-   ##
=======================================
  Coverage     76.2%   76.2%           
=======================================
  Files          760     760           
  Lines        61550   61568   +18     
  Branches      8125    8123    -2     
=======================================
+ Hits         46881   46905   +24     
+ Misses       14669   14663    -6

Files with missing lines	Coverage Δ
src/xrpld/app/rdb/backend/detail/Node.cpp	`58.4% <100.0%> (ø)`
src/xrpld/app/rdb/detail/Wallet.cpp	`78.6% <100.0%> (ø)`
src/xrpld/core/DatabaseCon.h	`100.0% <ø> (ø)`
src/xrpld/core/detail/DatabaseCon.cpp	`90.6% <100.0%> (+1.7%)`	⬆️
src/xrpld/app/rdb/detail/Vacuum.cpp	`0.0% <0.0%> (ø)`

... and 4 files with indirect coverage changes

vvysokikh1 · 2024-09-17T12:45:35Z

hey, I've been testing this PR and I can't make page_size work. It always sets it to 4096 with brand new transaction.db file. Does it work for you?

UPD: I think I've found the problem after debugging it for some time. Looks like the following code has an issue:

    template <std::size_t N, std::size_t M>
    DatabaseCon(
        boost::filesystem::path const& pPath,
        std::vector<std::string> const* commonPragma,
        std::array<std::string, N> const& pragma,
        std::array<char const*, M> const& initSQL,
        beast::Journal journal)
        : session_(std::make_shared<soci::session>()), j_(journal)
    {
        open(*session_, "sqlite", pPath.string());

        if (commonPragma)
        {
            for (auto const& p : *commonPragma) <<< some of the statement here creates a page in DB file, lockoing it to default 4096
            {
                soci::statement st = session_->prepare << p;
                st.execute(true);
            }
        }
        for (auto const& p : pragma) <<< This is the place where we set page_size
        {
            soci::statement st = session_->prepare << p;
            st.execute(true);
        }
        for (auto const& sql : initSQL)
        {
            soci::statement st = session_->prepare << sql;
            st.execute(true);
        }
    }

Changing the ordering of commonPragma and pragma helps to set the correct page_size. I'm not entirely sure this is correct solution though

UPD2: I understand this issue is not directly linked to your PR so I can take over the fix in this/other branch if you don't want to bother

vvysokikh1

As mentioned in previous comment, this change doesn't actually work. I've debugged the main problem and here're some nitpicks mostly