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

Issue #2570: DB schema change to make extensions opaque #2593

Merged
merged 6 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 8 additions & 4 deletions docs/db-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ homedomain | VARCHAR(44) | (BASE64)
thresholds | TEXT | (BASE64)
flags | INT NOT NULL |
lastmodified | INT NOT NULL | lastModifiedLedgerSeq
buyingliabilities | BIGINT CHECK (buyingliabilities >= 0) |
sellingliabilities | BIGINT CHECK (sellingliabilities >= 0) |
extension | TEXT | Extension specific to AccountEntry (XDR)
Copy link
Contributor

Choose a reason for hiding this comment

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

squash this commit into the commit with the schema change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

ledgerext | TEXT | Extension common to all LedgerEntry types (XDR)
signers | TEXT | (XDR)

## offers
Expand All @@ -73,6 +73,8 @@ priced | INT NOT NULL | Price.d
price | DOUBLE PRECISION NOT NULL | computed price n/d, used for ordering offers
flags | INT NOT NULL |
lastmodified | INT NOT NULL | lastModifiedLedgerSeq
extension | TEXT | Extension specific to OfferEntry (XDR)
ledgerext | TEXT | Extension common to all LedgerEntry types (XDR)
(offerid) | PRIMARY KEY |

## trustlines
Expand All @@ -91,8 +93,8 @@ tlimit | BIGINT NOT NULL DEFAULT 0 CHECK (tlimit >= 0) | limit
balance | BIGINT NOT NULL DEFAULT 0 CHECK (balance >= 0) |
flags | INT NOT NULL |
lastmodified | INT NOT NULL | lastModifiedLedgerSeq
buyingliabilities | BIGINT CHECK (buyingliabilities >= 0) |
sellingliabilities | BIGINT CHECK (sellingliabilities >= 0) |
extension | TEXT | Extension specific to TrustLineEntry (XDR)
ledgerext | TEXT | Extension common to all LedgerEntry types (XDR)
(accountid, issuer, assetcode) | PRIMARY KEY |

## accountdata
Expand All @@ -107,6 +109,8 @@ accountid | VARCHAR(56) NOT NULL | (STRKEY)
dataname | VARCHAR(88) NOT NULL | (BASE64)
datavalue | VARCHAR(112) NOT NULL | (BASE64)
lastmodified | INT NOT NULL | lastModifiedLedgerSeq
extension | TEXT | Extension specific to DataEntry (XDR)
ledgerext | TEXT | Extension common to all LedgerEntry types (XDR)
(accountid, dataname) | PRIMARY KEY |

## txhistory
Expand Down
218 changes: 217 additions & 1 deletion src/database/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
#include "main/Application.h"
#include "main/Config.h"
#include "overlay/StellarXDR.h"
#include "util/Decoder.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit (for the future): commit description is a bit verbose, just give a short summary. If a reader wants to get the details, they can look at the actual diff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll keep that in mind when I squash and re-split. Verbosity is a habit it might take me some time to break, though. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...I didn't actually shorten them much this time. Sorry about that.

#include "util/GlobalChecks.h"
#include "util/Logging.h"
#include "util/Timer.h"
#include "util/types.h"
#include <error.h>
#include <fmt/format.h>

#include "bucket/BucketManager.h"
#include "herder/HerderPersistence.h"
Expand All @@ -30,8 +33,10 @@
#include "medida/counter.h"
#include "medida/metrics_registry.h"
#include "medida/timer.h"
#include "xdr/Stellar-ledger-entries.h"

#include <lib/soci/src/backends/sqlite3/soci-sqlite3.h>
#include <string>
#ifdef USE_POSTGRES
#include <lib/soci/src/backends/postgresql/soci-postgresql.h>
#endif
Expand All @@ -57,7 +62,7 @@ bool Database::gDriversRegistered = false;

// smallest schema version supported
static unsigned long const MIN_SCHEMA_VERSION = 9;
static unsigned long const SCHEMA_VERSION = 12;
static unsigned long const SCHEMA_VERSION = 13;

// These should always match our compiled version precisely, since we are
// using a bundled version to get access to carray(). But in case someone
Expand Down Expand Up @@ -252,6 +257,33 @@ Database::applySchemaUpgrade(unsigned long vers)
// the accountbalances index around.
mSession << "DROP INDEX IF EXISTS accountbalances";
break;
case 13:
Copy link
Contributor

Choose a reason for hiding this comment

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

let's open an issue to collapse all schemas older than 12 into just 12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done; see issue #2601 .

if (!mApp.getConfig().MODE_USES_IN_MEMORY_LEDGER)
{
// Add columns for the LedgerEntry extension to each of
// the tables that stores a type of ledger entry.
addTextColumn("accounts", "ledgerext");
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like this commit also contains changes for the previous commit (cleanup?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, commits have been cleaned up.

addTextColumn("trustlines", "ledgerext");
addTextColumn("accountdata", "ledgerext");
addTextColumn("offers", "ledgerext");
// Absorb the explicit columns of the extension fields of
// AccountEntry and TrustLineEntry into single opaque
// blobs of XDR each of which represents an entire extension.
convertAccountExtensionsToOpaqueXDR();
convertTrustLineExtensionsToOpaqueXDR();
// Neither earlier schema versions nor the one that we're upgrading
// to now had any extension columns in the offers or accountdata
// tables, but we add columns in this version, even though we're not
// going to use them for anything other than writing out opaque
// base64-encoded empty v0 XDR extensions, so that, as with the
// other LedgerEntry extensions, we'll be able to add such
// extensions in the future without bumping the database schema
// version, writing any upgrade code, or changing the SQL that reads
// and writes those tables.
addTextColumn("offers", "extension");
addTextColumn("accountdata", "extension");
}
break;
default:
throw std::runtime_error("Unknown DB schema version");
}
Expand All @@ -277,6 +309,7 @@ Database::upgradeToCurrentSchema()
std::to_string(SCHEMA_VERSION));
throw std::runtime_error(s);
}
actBeforeDBSchemaUpgrade();
while (vers < SCHEMA_VERSION)
{
++vers;
Expand All @@ -289,6 +322,189 @@ Database::upgradeToCurrentSchema()
assert(vers == SCHEMA_VERSION);
}

void
Database::addTextColumn(std::string const& table, std::string const& column)
{
std::string addColumnStr("ALTER TABLE " + table + " ADD " + column +
" TEXT;");
CLOG(INFO, "Database") << "Adding column '"
<< "' to table '" << table << "'";
mSession << addColumnStr;
}

void
Database::dropNullableColumn(std::string const& table,
std::string const& column)
{
// SQLite doesn't give us a way of dropping a column with a single
// SQL command. If we need it in production, we could re-create the
// table without the column and drop the old one. Since we currently
// use SQLite only for testing and PostgreSQL in production, we simply
// leave the unused columm around in SQLite at the moment, and NULL
// out all of the cells in that column.
if (!isSqlite())
{
std::string dropColumnStr("ALTER TABLE " + table + " DROP COLUMN " +
column);
CLOG(INFO, "Database") << "Dropping column '" << column
<< "' from table '" << table << "'";

mSession << dropColumnStr;
}
else
{
std::string nullColumnStr("UPDATE " + table + " SET " + column +
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment to above. For accounts and trustlines, this is going to write a little less than 10 million rows. Is the performance of this bearable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above (the ~2m and ~20s figures are for the complete upgrade, including the filling of all the new columns in the first case).

" = NULL");
CLOG(INFO, "Database") << "Setting all cells of column '" << column
<< "' in table '" << table << "' to NULL";

mSession << nullColumnStr;
}
}

std::string
Database::getOldLiabilitySelect(std::string const& table,
std::string const& fields)
{
return fmt::format("SELECT {}, "
"buyingliabilities, sellingliabilities FROM {} WHERE "
"buyingliabilities IS NOT NULL OR "
"sellingliabilities IS NOT NULL",
fields, table);
}

void
Database::convertAccountExtensionsToOpaqueXDR()
{
addTextColumn("accounts", "extension");
copyIndividualAccountExtensionFieldsToOpaqueXDR();
dropNullableColumn("accounts", "buyingliabilities");
dropNullableColumn("accounts", "sellingliabilities");
}

void
Database::convertTrustLineExtensionsToOpaqueXDR()
{
addTextColumn("trustlines", "extension");
copyIndividualTrustLineExtensionFieldsToOpaqueXDR();
dropNullableColumn("trustlines", "buyingliabilities");
dropNullableColumn("trustlines", "sellingliabilities");
}

void
Database::copyIndividualAccountExtensionFieldsToOpaqueXDR()
{
std::string const tableStr = "accounts";

CLOG(INFO, "Database") << "Updating extension schema for " << tableStr;

// <accountID, extension>
struct Fields
{
std::string mAccountID;
std::string mExtension;
};

std::string const fieldsStr = "accountid";
std::string const selectStr = getOldLiabilitySelect(tableStr, fieldsStr);
auto makeFields = [](soci::row const& row) {
AccountEntry::_ext_t extension;
// getOldLiabilitySelect() places the buying and selling extension
// column names after the key field in the SQL select string.
extension.v(1);
extension.v1().liabilities.buying = row.get<long long>(1);
extension.v1().liabilities.selling = row.get<long long>(2);
return Fields{.mAccountID = row.get<std::string>(0),
.mExtension =
decoder::encode_b64(xdr::xdr_to_opaque(extension))};
};

std::string const updateStr =
"UPDATE accounts SET extension = :ext WHERE accountID = :id";
auto prepUpdate = [](soci::statement& st_update, Fields const& data) {
st_update.exchange(soci::use(data.mExtension)),
st_update.exchange(soci::use(data.mAccountID));
};

auto postUpdate = [](long long const affected_rows, Fields const& data) {
if (affected_rows != 1)
{
throw std::runtime_error(fmt::format(
"{}: updating account with account ID {} affected {} row(s) ",
__func__, data.mAccountID, affected_rows));
}
};

size_t numUpdated = selectUpdateMap<Fields>(
*this, selectStr, makeFields, updateStr, prepUpdate, postUpdate);

CLOG(INFO, "Database") << __func__ << ": updated " << numUpdated
<< " records(s) with liabilities in " << tableStr
<< " table";
}

void
Database::copyIndividualTrustLineExtensionFieldsToOpaqueXDR()
{
std::string const tableStr = "trustlines";

CLOG(INFO, "Database") << __func__ << ": updating extension schema for "
<< tableStr;

// <accountID, issuer_id, asset_id, extension>
struct Fields
{
std::string mAccountID;
std::string mIssuerID;
std::string mAssetID;
std::string mExtension;
};

std::string const fieldsStr = "accountid, issuer, assetcode";
std::string const selectStr = getOldLiabilitySelect(tableStr, fieldsStr);
auto makeFields = [](soci::row const& row) {
TrustLineEntry::_ext_t extension;
// getOldLiabilitySelect() places the buying and selling extension
// column names after the three key fields in the SQL select string.
extension.v(1);
extension.v1().liabilities.buying = row.get<long long>(3);
extension.v1().liabilities.selling = row.get<long long>(4);
return Fields{.mAccountID = row.get<std::string>(0),
.mIssuerID = row.get<std::string>(1),
.mAssetID = row.get<std::string>(2),
.mExtension =
decoder::encode_b64(xdr::xdr_to_opaque(extension))};
};

std::string const updateStr =
"UPDATE trustlines SET extension = :ext WHERE accountID = :id "
"AND issuer = :issuer_id AND assetcode = :asset_id";
auto prepUpdate = [](soci::statement& st_update, Fields const& data) {
st_update.exchange(soci::use(data.mExtension));
st_update.exchange(soci::use(data.mAccountID));
st_update.exchange(soci::use(data.mIssuerID));
st_update.exchange(soci::use(data.mAssetID));
};

auto postUpdate = [](long long const affected_rows, Fields const& data) {
if (affected_rows != 1)
{
throw std::runtime_error(fmt::format(
"{}: updating trustline with account ID {}, issuer {}, and "
"asset {} affected {} row(s)",
__func__, data.mAccountID, data.mIssuerID, data.mAssetID,
affected_rows));
}
};

size_t numUpdated = selectUpdateMap<Fields>(
*this, selectStr, makeFields, updateStr, prepUpdate, postUpdate);

CLOG(INFO, "Database") << __func__ << ": updated " << numUpdated
<< " records(s) with liabilities in " << tableStr
<< " table";
}

void
Database::putSchemaVersion(unsigned long vers)
{
Expand Down
Loading