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

rokopt · 2020-06-23T01:32:59Z

Update DB schema to make all ledger entry extensions opaque

Resolves #2570

This pull request changes the database schema as proposed in issue #2570 by converting the extension fields of AccountEntry, TrustLineEntry, DataEntry, OfferEntry, and LedgerEntry into opaque XDR. It upgrades old databases by copying the existing transparent extensions (which are Liabilities fields in the case of AccountEntry and TrustLineEntry, and empty in the other cases) into new columns, and then deleting the column containing the transparent extensions. (Extensions that were empty had no old columns; they simply gain new ones.)

The PR introduces a test which creates a mechanism for injecting code into points during application creation, in this case, between the creation of the database (which we do with an old schema) or the loading of an old database and the upgrading of the database to the new schema. It injects code to use raw SQL to create ledger entries of all types while the database is in the old format. Then, after the application initialization has completed, and the database has been upgraded, it uses the standard interfaces to load the accounts, trustlines, key-value data, and offers, and validate that they have been upgraded as expected (accounts or trustlines which had NULL Liabilities fields will now have NULL extension fields, while those which had non-NULL Liabilities will have non-NULL opaque extensions which decode to Liabilities fields equal to the old ones).

See #2584 for conversations about a previous version of this pull request, which is made obsolete by this one (which has had its history cleaned up, as well as having had extensive revisions since PR 2584).

I'm starting out by filing this PR as a draft because I still have to do some performance investigations on large, real-world databases; possible questions include:

How long does upgrade take on the real database on a system such as owlbear?
Does live performance on the real database change measurably after an upgrade on a system such as owlbear?
Does PostgreSQL collation make any difference to live performance? (Or does that only help with fields used as keys?)
How long would a full database rewrite take as opposed to the column upgrade done by this code, and would there be a difference between the two in live performance afterwards? (And if, for example, the full rewrite would take much longer, but would also produce better performance afterwards, is there a way that people can choose to take the time to do a full rewrite if they wish to?)

Checklist

Reviewed the contributing document
Rebased on top of master (no merge commits)
Ran clang-format v5.0.0 (via make format or the Visual Studio extension)
Compiles
Ran all tests
If change impacts performance, include supporting evidence per the performance document

rokopt · 2020-06-23T19:47:38Z

There must be something that I'm failing to do during the upgrade. I can create a pubnet database with schema 12, run on it for a while, and then upgrade it with my new code. And I can create a new database with schema 13 with my new code, and run on that. But if I run on schema 12 for a while and then upgrade with my new code, I can't run with the upgraded database; this happens on startup:

2020-06-23T12:42:07.369 GDJYT [Ledger INFO] Last closed ledger (LCL) hash is 78d269aaddb5f15a51c56873e2178168a3b57eff3cdbfcbccc5faa0b7f6fc22b
2020-06-23T12:42:07.370 GDJYT [Ledger INFO] Loaded LCL header from database: [seq=30215056, hash=78d269]
2020-06-23T12:42:07.371 GDJYT [Ledger ERROR] 30 buckets are missing from bucket directory 'buckets' [LedgerManagerImpl.cpp:329]
2020-06-23T12:42:07.372 GDJYT [default FATAL] Got an exception: Bucket directory is corrupt [ApplicationUtils.cpp:75]

rokopt · 2020-06-23T20:23:32Z

There must be something that I'm failing to do during the upgrade. I can create a pubnet database with schema 12, run on it for a while, and then upgrade it with my new code. And I can create a new database with schema 13 with my new code, and run on that. But if I run on schema 12 for a while and then upgrade with my new code, I can't run with the upgraded database; this happens on startup:
2020-06-23T12:42:07.369 GDJYT [Ledger INFO] Last closed ledger (LCL) hash is 78d269aaddb5f15a51c56873e2178168a3b57eff3cdbfcbccc5faa0b7f6fc22b
2020-06-23T12:42:07.370 GDJYT [Ledger INFO] Loaded LCL header from database: [seq=30215056, hash=78d269]
2020-06-23T12:42:07.371 GDJYT [Ledger ERROR] 30 buckets are missing from bucket directory 'buckets' [LedgerManagerImpl.cpp:329]
2020-06-23T12:42:07.372 GDJYT [default FATAL] Got an exception: Bucket directory is corrupt [ApplicationUtils.cpp:75]

Oh, I think that was operator rather than code error on my part -- I did the two runs (with the old code and then the new code) from two different working directories, and I think that by default the "buckets" directory is placed in the current working directory. So I was using the wrong buckets directory for the run with the new code.

rokopt · 2020-06-24T15:25:30Z

There must be something that I'm failing to do during the upgrade. I can create a pubnet database with schema 12, run on it for a while, and then upgrade it with my new code. And I can create a new database with schema 13 with my new code, and run on that. But if I run on schema 12 for a while and then upgrade with my new code, I can't run with the upgraded database; this happens on startup:
2020-06-23T12:42:07.369 GDJYT [Ledger INFO] Last closed ledger (LCL) hash is 78d269aaddb5f15a51c56873e2178168a3b57eff3cdbfcbccc5faa0b7f6fc22b
2020-06-23T12:42:07.370 GDJYT [Ledger INFO] Loaded LCL header from database: [seq=30215056, hash=78d269]
2020-06-23T12:42:07.371 GDJYT [Ledger ERROR] 30 buckets are missing from bucket directory 'buckets' [LedgerManagerImpl.cpp:329]
2020-06-23T12:42:07.372 GDJYT [default FATAL] Got an exception: Bucket directory is corrupt [ApplicationUtils.cpp:75]
Oh, I think that was operator rather than code error on my part -- I did the two runs (with the old code and then the new code) from two different working directories, and I think that by default the "buckets" directory is placed in the current working directory. So I was using the wrong buckets directory for the run with the new code.

Confirmed, that was my operator error, using different bucket directories for the two runs.

rokopt · 2020-06-24T15:29:13Z

Performance note:

After running overnight on the pubnet with DB schema v12 (prior to this PR), I switched over to v13 (a binary with the changes from this PR), and, as it consistently has so far when I've tested upgrades on owlbear, it took ~20s to upgrade the database schema, which involved rewriting ~60K accounts records and ~120K trustlines records (that's how many have liabilities in the current database). Including that time and the ensuing catchup, it took ~2m40s for the node to sync back up on v13.

jonjove

Overall, this looks good. I have a variety of comments, many of which are similar in substance. I had so many comments on the tests that I stopped reviewing them and will review them again after they are updated.

src/main/Application.h

src/database/Database.h

src/database/Database.cpp

src/database/Database.h

jonjove · 2020-06-23T15:15:23Z

src/database/Database.cpp

+            // extensions in the future without bumping the database schema
+            // version, writing any upgrade code, or changing the SQL that reads
+            // and writes those tables.
+            addTextColumnWithDefault(


I'm not intrinsically opposed to doing this for accountdata and offers. But as it stands today, it does not look likely that those XDR types will be extended any time in the near future (I've yet to encounter a proposal that would extend either). There is a minor overhead in storage and performance. @MonsieurNicolas what do you think?

Now that the new columns aren't being filled, but will just be NULL, perhaps that overhead will be less? (Of course, I'll implement whatever you and @MonsieurNicolas decide you prefer.)

src/database/test/DatabaseTests.cpp

docs/db-schema.md

src/database/Database.cpp

rokopt · 2020-06-25T02:14:01Z

I think I've addressed all the review comments so far, mostly by just making the suggested changes, and in a few cases proposing and implementing different ones or offering reasons to leave it as it is. I've finally marked this as ready for review, as opposed to a draft, though further review and discussion will almost certainly lead to some further changes.

rokopt · 2020-06-25T05:28:33Z

Here's a raw benchmark log from owlbear, comparing a series of ledger-entry-related [bench] tests, with a v12 run first and then a v13 run. I have not done enough to get an idea of the variance, but at a first glance nothing has jumped out at me as untoward.

benchmarks-v12-vs-v13.txt

src/database/test/DatabaseTests.cpp

src/database/Database.cpp

src/database/Database.h

jonjove · 2020-06-25T14:57:03Z

src/main/ApplicationImpl.h

@@ -116,6 +116,8 @@ class ApplicationImpl : public Application

    virtual AbstractLedgerTxnParent& getLedgerTxnRoot() override;

+    virtual void actBeforeDBSchemaUpgrade() override;


Do you think this should be a virtual function of Database instead of Application? Seems like separation of concerns.

The idea was that Application would be the one class which dictated where its clients could inject code. If actBeforeDBSchemaUpgrade() were to move to the Database class, how are you envisioning clients creating Application instances that injected code into Database methods?

I would add a virtual function ApplicationImpl::createDatabase (see createInvariantManager, createOverlayManager, and createOverlayManager for reference) and overload this in the derived class. Then you could use basically the same injection mechanism as you have here, where the derived Application takes a functor which passes it to the derived Database upon construction, and the functor is called before schema upgrade.

I admit it's a bit less direct. It has the disadvantage of requiring a bit more boiler-plate. But it has the advantage of having better separation of concerns and being more composable. I'm not sure how much it matters here, but I figured we should at least discuss it.

Aha -- I had briefly mulled over trying to work out how to do that, but I was afraid it would come across as too heavyweight, and didn't get as far as noticing that we had precedent!

Though the result is more verbose, "having better separation of concerns and being more composable" is music to my ears, so I've taken your suggestion. Does the result look sane?

rokopt · 2020-06-25T17:56:46Z

I'm attaching a couple "metrics" collections from running for a while on each of v12 and v13. This is my first time trying to interpret our metrics, but after looking over the database.* and ledger.* ones that appear to be measuring the database operations that involve the tables that I've changed, I'm not seeing anything that looks like a statistically
significant regression.

metrics-v12-long-run.txt
metrics-v13-long-run.txt

rokopt · 2020-06-25T22:38:03Z

Jon pointed out that, although we mostly expect PostgreSQL to be our production-use database, we're not sure that no one's using SQLite in production, and the latter behaves differently during the upgrade because it doesn't let us drop columns, so we NULL out that field of all rows instead, which is a much larger operation than anything we do in the PostgreSQL upgrade. Therefore, I'd better test the SQLite upgrade on the real pubnet database too.

I've now done so, and on owlbear, the SQLite schema upgrade took 2m13s. I'd happened to guess based on other measurements that it would be somewhere around 2m, and asked Nicolas whether it would be alright if it were, and he said that that would be fine, so I am venturing to hope that 2m13s is fine too.

rokopt · 2020-06-26T01:40:29Z

Jon pointed out that catchup performance would provide a strongly apples-to-apples comparison between v12 and v13, since it would involve applying exactly the same buckets; he suggested the command stellar-core catchup 30000000/10000. I tried that on owlbear using PostgreSQL, and v13 was ~20s slower out of a ~65m test (0.5%). I haven't repeated the test to try to get an idea of the variance yet, but even if that's significant, I imagine it's probably worthwhile for the future flexibility.

catchup-perf-v12.txt
catchup-perf-v13.txt

rokopt · 2020-06-26T04:12:02Z

Commits have been re-squashed. ...Non-destructively, I hope.

MonsieurNicolas

I didn't find actual bugs, good job.

Mostly cosmetic issues from what I see.

In particular, the commit history is quite hard to follow as it's only partially squashed.

Given the changes, I think it's probably easier to squash everything and resplit instead of propagating a subset of changes across commits (but you can try to do this as an exercise if you've never done it).

MonsieurNicolas · 2020-06-28T00:17:50Z

src/database/Database.cpp

+{
+    auto st_update = getPreparedStatement(updateStr).statement();
+
+    for (auto recT : in)


auto & recT

MonsieurNicolas · 2020-06-28T00:24:49Z

src/database/Database.cpp

@@ -252,6 +258,33 @@ Database::applySchemaUpgrade(unsigned long vers)
        // the accountbalances index around.
        mSession << "DROP INDEX IF EXISTS accountbalances";
        break;
+    case 13:


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

Done; see issue #2601 .

MonsieurNicolas · 2020-06-28T00:26:49Z

src/database/Database.cpp

@@ -9,10 +9,13 @@
 #include "main/Application.h"
 #include "main/Config.h"
 #include "overlay/StellarXDR.h"
+#include "util/Decoder.h"


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

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. ;)

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

MonsieurNicolas · 2020-06-28T00:28:52Z

src/database/Database.cpp

+        {
+            // Add columns for the LedgerEntry extension to each of
+            // the tables that stores a type of ledger entry.
+            addTextColumn("accounts", ledgerExtName);


you forgot to update schema.md to match this latest schema

I think all the changes got reflected in my later commit to db-schema.md, right?

MonsieurNicolas · 2020-06-28T00:31:52Z

src/database/Database.cpp

+{
+    std::string addColumnStr("ALTER TABLE " + table + " ADD " + column +
+                             " TEXT;");
+    CLOG(INFO, "Database") << "Adding column with string '" << addColumnStr


nit: why doesn't it just say "Adding column '" << column << "'" ?

This is a general problem in this file: you're logging "how the sausage is made", which is too verbose for normal operators (and nobody really cares about those details)

Okay. I did it that way because I occasionally wrote syntax errors and wanted to see exactly what I'd done wrong (the soci exceptions generally don't say). But it makes sense that for a final version, operators wouldn't care.

I've updated the log messages in that file to try to constrain them to "what" and not "how".

MonsieurNicolas · 2020-06-28T01:10:57Z

src/ledger/LedgerTxnDataSQL.cpp

+            "ON CONFLICT (accountid, dataname) DO UPDATE SET "
+            "datavalue = excluded.datavalue, "
+            "lastmodified = excluded.lastmodified, "
+            "extension = excluded.extension, ledgerext "


same here ledgerext = excluded.ledgerext

MonsieurNicolas · 2020-06-28T01:11:45Z

src/ledger/LedgerTxnOfferSQL.cpp

+            "price = excluded.price, "
+            "flags = excluded.flags, "
+            "lastmodified = excluded.lastmodified, "
+            "extension = excluded.extension, ledgerext "


ledgerext = excluded.ledgerext

MonsieurNicolas · 2020-06-28T01:12:51Z

src/main/Application.h

@@ -278,8 +278,10 @@ class Application

    virtual AbstractLedgerTxnParent& getLedgerTxnRoot() = 0;

-    // For tests to perform operations on databases while they're
-    // still using old schemas.
+    // Give clients the opportunity to perform operations on databases while


wrong commit

Fixed, I think.

MonsieurNicolas · 2020-06-28T01:17:53Z

src/database/Database.h

@@ -214,6 +218,16 @@ class Database : NonMovableOrCopyable
    // Access the optional SOCI connection pool available for worker
    // threads. Throws an error if !canUsePool().
    soci::connection_pool& getPool();
+
+  protected:
+    // Give clients the opportunity to perform operations on databases while


the application test hook that you removed here should just be removed from history as you never used it (it's just extra commits and churn for no reason)

MonsieurNicolas · 2020-06-28T01:20:19Z

docs/db-schema.md

@@ -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)


squash this commit into the commit with the schema change

rokopt · 2020-06-28T03:00:22Z

I didn't find actual bugs, good job.

Mostly cosmetic issues from what I see.

In particular, the commit history is quite hard to follow as it's only partially squashed.

Given the changes, I think it's probably easier to squash everything and resplit instead of propagating a subset of changes across commits (but you can try to do this as an exercise if you've never done it).

Yeah, I wasn't sure what I should do about later commits that affected multiple previous ones. I wondered whether I should try to squash them all into one and then split them up again. Should I do that before merge?

Add a mechanism, using the Factory pattern, for production code to create injection points that can call back into test-defined code (or perhaps other applications in the future; parameters used by the injected code can vary with each test or other use, including, in effect, the method called, by the use of lambdas). A test client can create one derived class of Database for each injection point or combination of injection points (into methods of class Database -- other classes could also use this mechanism for injecting code into their methods) to override with methods that do something. The derived class's constructor may take any list of parameter types; each call to createTestApplication() (or Application::create()) passes its own list of parameters of those types. The derived class may store the parameters received by its constructor, so that it is effectively called with parameters even though the production code calls a method with no parameters. This means in particular that TestApplication itself does not need to acquire new members to allow tests to store parameters to injection point methods. A single derived class of Database may be used by multiple test cases even to call, in effect, different injected methods, because the parameters passed by createTestApplication() to the constructor of the derived class of TestApplication may include lambdas. This checkin also adds one use of the new mechanism, actBeforeDBSchemaUpgrade(), by production code to create an injection point.

Introduce three generic functions for operating on database records in bulk: - selectMap() takes a function which maps a SQL record to an element of a client-defined (templated) type, and produces a function which takes a SQL SELECT statement and produces a vector of the client-defined type comprising the list of selected records mapped through the client-defined function. - updateMap() takes a function which maps an element of a client-defined (templated) type to a SQL UPDATE statement, and produces a function which generates a series of SQL UPDATE statements from a vector of the client-defined type. - selectUpdateMap() produces the composition of updateMap() following selectMap(), namely, a function which takes a SQL SELECT statement and produces for each selected record an UPDATE statement derived from a transformation applied to the record via client-defined functions which interpret the record as an element of a client-defined type and then use that interpretation to choose some update to perform on it.

Upgrade the DB schema from 12 to 13. The new schema makes all of the extensions in all ledger entry types opaque to the database code itself. Those extensions are the extension to LedgerEntry itself, which is stored in the table for each LedgerEntry type (account, trustline, data, offer), and the extensions which are specific to each type. The idea of making the extensions opaque is that they can be extended in the future without further changes to either the DB schema or the DB code, as the database itself will simply continue to treat the extensions as variable-length, opaque XDR. The cost is that we can't query on components of the extensions (which we don't do currently, and won't do with any of the extensions that we currently have planned). The general LedgerEntry extension, as well as the extensions to DataEntry and OfferEntry, are currently unused -- they are unions with the only version being 0 and no content besides that fixed version number, and they are not stored in any database columns at all. The upgrade code therefore simply creates columns for them and populates all of the cells of the new columns with base-64 encodings of opaque XDR representing that trivial tagged union. The two extensions that are already used are those specific to AccountEntry and TrustLineEntry, both of which contain the same thing, namely a Liabilities structure (which contains two fields, one for buying and one for selling). Those extensions are tagged unions with a genuine version option -- version 0 for entries that have never yet had liabilities, and then version 1 when liabilities are first created for the entry. Existing databases have NULL in the "buyingliabilities" and "sellingliabilities" columns for the version-0 entries, and non-NULL values, namely base-64 encodings of the XDR for the Liabilities structure, for the version-1 entries. Therefore, the upgrade code for AccountEntry and TrustLineEntry performs a conversion: for each record, it reads the "buyingliabilities" and "sellingliabilities" columns, which are 64-bit integers transparent to the database, and generates encoded XDR, opaque to the database, representing an extension field which, in those cases, has a Liabilities field, and has the values read from the old columns filled in. (If the old liabilities fields for a record are NULL, then the new "extension" column for that record is also NULL -- the new live database code will recognize that NULL values of those two extensions are possible.) Once all of the records have had their new "extension" columns written, the upgrade code deletes the old "buyingliabilities" and "sellingliabilities" columns.

Modify the live database code for all of the LedgerEntry types -- AccountEntry, TrustLineEntry, DataEntry, and OfferEntry -- to use database schema 13, in which the extensions for each of those individual entry types, as well as the extension for LedgerEntry itself which is also stored in the table for whatever specific type a given LedgerEntry has, are just base-64-encoded opaque XDR as far as the database is concerned, regardless of how the ledger-transaction code inteprets those extensions, now or in the future.

Test upgrades to database schema 13, in which all extensions in all ledger entry types become opaque XDR as far as the database is concerned. The tests use the test-code-injection mechanism which allow them to run arbitrary code between the creation of a database (which is done in an old schema version, MIN_SCHEMA_VERSION) and the upgrading of that database to the current schema version (SCHEMA_VERSION). They create ledger entries of each of the four types, using raw SQL so that they can create them in the old format which the live production code no longer uses. After the database upgrade completes (which it has done by the time that createTestApplication() returns), the tests validate that the contents of the ledger entries are intact and logically unchanged (although their representations in the database have changed, with some columns having been created and some having been deleted, and some contents being encoded differently).

rokopt · 2020-06-29T16:56:38Z

Given the changes, I think it's probably easier to squash everything and resplit instead of propagating a subset of changes across commits (but you can try to do this as an exercise if you've never done it).

Done; commits have been squashed and re-split.

MonsieurNicolas · 2020-07-07T00:19:13Z

r+ 5801f31

rokopt mentioned this pull request Jun 23, 2020

Issue 2570 pr 1 #2584

Closed

6 tasks

jonjove suggested changes Jun 24, 2020

View reviewed changes

rokopt marked this pull request as ready for review June 25, 2020 02:12

rokopt requested a review from jonjove June 25, 2020 02:14

jonjove reviewed Jun 25, 2020

View reviewed changes

rokopt mentioned this pull request Jun 25, 2020

Factor out raw SQL into dedicated module (Version: [v13.1.0]) #2594

Open

rokopt requested a review from jonjove June 25, 2020 17:30

rokopt force-pushed the issue-2570-pr-4 branch 2 times, most recently from 16b6814 to 5ac37ea Compare June 26, 2020 03:45

MonsieurNicolas mentioned this pull request Jun 27, 2020

CAP-0023 implementation #2591

Merged

6 tasks

MonsieurNicolas requested changes Jun 28, 2020

View reviewed changes

rokopt force-pushed the issue-2570-pr-4 branch 2 times, most recently from 4676839 to 669bb99 Compare June 29, 2020 15:44

rokopt added 5 commits June 29, 2020 09:20

Utilities for decoding base-64-encoded opaque XDR

0af8465

rokopt force-pushed the issue-2570-pr-4 branch from 669bb99 to 5801f31 Compare June 29, 2020 16:54

rokopt requested a review from MonsieurNicolas June 29, 2020 17:03

latobarita merged commit 7ab017b into stellar:master Jul 7, 2020

rokopt deleted the issue-2570-pr-4 branch August 3, 2020 16:19

tamirms mentioned this pull request Sep 16, 2020

Dump Ledger State Job is broken due to Stellar Core schema changes stellar/go#3022

Closed

nebolsin mentioned this pull request Nov 2, 2020

Ensure compatibility with stellar-core 15.x astroband/astrograph#207

Open

		@@ -116,6 +116,8 @@ class ApplicationImpl : public Application

		virtual AbstractLedgerTxnParent& getLedgerTxnRoot() override;

		virtual void actBeforeDBSchemaUpgrade() override;

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

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

Conversation

rokopt commented Jun 23, 2020 • edited Loading

Update DB schema to make all ledger entry extensions opaque

Checklist

rokopt commented Jun 23, 2020 • edited Loading

rokopt commented Jun 23, 2020

rokopt commented Jun 24, 2020

rokopt commented Jun 24, 2020

jonjove left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rokopt commented Jun 25, 2020

rokopt commented Jun 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rokopt commented Jun 25, 2020

rokopt commented Jun 25, 2020

rokopt commented Jun 26, 2020

rokopt commented Jun 26, 2020

MonsieurNicolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rokopt commented Jun 28, 2020

rokopt commented Jun 29, 2020

MonsieurNicolas commented Jul 7, 2020

rokopt commented Jun 23, 2020 •

edited

Loading

rokopt commented Jun 23, 2020 •

edited

Loading