compaction test txlogs #1780

chessai · 2023-11-15T05:44:08Z

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1205378300254932

…d version

chessai · 2023-11-16T05:02:23Z

test/Chainweb/Test/Pact/RemotePactTest.hs

@@ -135,6 +147,12 @@ tests rdb = testGroup "Chainweb.Test.Pact.RemotePactTest"
        withResource' getCurrentTimeIntegral $ \(iotm :: IO (Time Micros)) ->
            let cenv = _getServiceClientEnv <$> net
                iot = toTxCreationTime <$> iotm
+                pactDir = do
+                  m <- _getNodeDbDirs <$> net
+                  case M.lookup 0 m of


This looks up the pactDbDir for node 0. This is kind of a hack, because there is only one node in this test. However, it doesn't matter much, because we are dealing with both submitting /local txs and compaction, so picking an arbitrary node to run these two operations on is fine.

Why don't we just do withNode instead of withNodes?

I have no idea. I think this test suite (RemotePactTest and some others) were originally meant to be flexible enough to run multiple nodes, but they don't. AFAIK only the slow-tests (MultiNode.hs) actually run multiple nodes.

I don't want to make the change in this PR though.

chessai · 2023-11-16T16:11:59Z

test/Chainweb/Test/Pact/RemotePactTest.hs

 -- -------------------------------------------------------------------------- --
 -- Global Settings

-nNodes :: Natural
+nNodes :: Word


you'll see a few similar changes throughout this PR. there's no reason for this to be a Natural. we're never going to run greater than maxBound :: Word nodes on a single machine, or even come close to approaching it.

test/Chainweb/Test/Pact/RemotePactTest.hs

chessai · 2023-11-16T16:14:39Z

test/Chainweb/Test/Pact/RemotePactTest.hs

@@ -948,5 +1187,5 @@ pactDeadBeef = let (TransactionHash b) = deadbeef
               in RequestKey $ Hash b

 -- avoiding `scientific` dep here
-getBlockHeight :: CommandResult a -> Maybe Word64
-getBlockHeight = preview (crMetaData . _Just . key "blockHeight" . _Number . to ((fromIntegral :: Integer -> Word64 ) . round . toRational))
+crGetBlockHeight :: CommandResult a -> Maybe Word64


getBlockHeight is a bit ambiguously named. Get it from what?
The cr prefix makes things a bit clearer, IMO.
I made this change initially because I had defined a function for testing called getBlockHeight, and was surprised to see a conflict for a function doing something entirely different.

chessai · 2023-11-16T16:16:32Z

test/Chainweb/Test/Utils.hs

@@ -1043,6 +1054,33 @@ node testLabel rdb rawLogger peerInfoVar conf nid = do
        crs = map snd $ HashMap.toList $ view chainwebChains cw
        poison cr = mempoolAddToBadList (view chainResMempool cr) (V.singleton deadbeef)

+withDbDirs :: Word -> ResourceT IO (Map Word (FilePath, FilePath))


the code to create+destroy the temporary dirs was copied mostly from System.IO.Temp.withSystemTempDirectory, but we needed a Map version (each node needs its own directories)

thoughtpolice

readability: lgtm

test/Chainweb/Test/Utils.hs

edmundnoble · 2023-11-16T17:13:10Z

test/Chainweb/Test/Utils.hs

        -- ^ Unique Node Id. The node id 0 is used for the bootstrap node
    -> IO ()
-node testLabel rdb rawLogger peerInfoVar conf nid = do
+node testLabel rdb rawLogger peerInfoVar conf pactDbDir rocksDbDir nid = do


Idk if this is the best PR to do it, but I think withChainweb should not take these directories as an input. ChainwebConfiguration contains a database directory, and that should be the base directory for all databases. That further means that node stops needing to take them as input because it takes a ChainwebConfiguration as well.

yeah, i don't think this PR should do it. we should make a ticket.

edmundnoble · 2023-11-16T17:29:13Z

test/Chainweb/Test/Pact/RemotePactTest.hs

@@ -135,6 +147,12 @@ tests rdb = testGroup "Chainweb.Test.Pact.RemotePactTest"
        withResource' getCurrentTimeIntegral $ \(iotm :: IO (Time Micros)) ->
            let cenv = _getServiceClientEnv <$> net
                iot = toTxCreationTime <$> iotm
+                pactDir = do
+                  m <- _getNodeDbDirs <$> net
+                  case M.lookup 0 m of


Why don't we just do withNode instead of withNodes?

test/Chainweb/Test/Pact/RemotePactTest.hs

edmundnoble · 2023-11-16T20:05:48Z

chainweb.cabal

@@ -509,7 +509,7 @@ test-suite chainweb-tests

    build-depends:
        -- internal
-          chainweb
+        , chainweb


i prefer leading commas for easy re-arranging (eg with vim's :sort u), but i can undo this. originally it was because i added a temporary dependency above this

* PactDB: compute sha3 checksums * fix build * compact system tables * refactor * docs * test verifies hashes * return global hash * dedicated test * Code-complete with cw-tool * test comments * cleanup sql * cleanup * cleanup * fix build * Hashing no longer in checkpointer, internal to compaction * cleanup * fix drop table, add vacuum * test TODOs * optparse cleanups * remove flag addition in chainweb.cabal * clean up imports * make relevant fields strict * minor src cleanups * add some docs to compaction functions * first test stuff * more compaction haddocks/add blockheight to invalid block height exception * remove redundant import * add flag to not drop new tables * compact concurrently, add log directory for per-chain logs * debugging doRestore * compaction: just don't drop new tables at all, ever * change compact log file name prefix from compact-log- to compact-chain- * Revert "compaction: just don't drop new tables at all, ever" This reverts commit 06210f1. * undo residual changes from rebase * undo redundant logging * use new chainweb version code * print compaction hash in base64 * fix compactSystemTables * Add --threads option and use pooledMapConcurrentlyN_ in compaction * compaction logging: create directory and files before attempting to log. not sure why this stopped working? * add new test idea from John * add NoGrandHash flag * fix type error * fix compact flag for no grand hash * don't verifyTable when NoGrandHash is set * fix compile error in tests from prior change * make PactService exit if rosetta is enabled and we don't have the full history (#1736) * make PactService exit if rosetta is enabled and we don't have the full history * Compaction.hs: delete extra newline * PactService: temporarily putStrLn some crude output for testing purposes * refactor withPactTestBlockDb to give access to the SQLiteEnv * doGetEarliest: fix callDb debug string * fix tests * _pactRosettaEnabled -> _pactFullHistoryRequired * informative error message if you try to rewind before min(blockheight) (#1726) * informative error message if you try to rewind before min(blockheight) * earliestBlock returning nothing should use genesisHeight * attempt FastTimedCPM10to19; failure * Revert "attempt FastTimedCPM10to19; failure" This reverts commit 66c807b. * amend test * add test * remove new exception type and revert changes in failOnTooLowRequestedHeight * remove redundant test in PactMultiChainTest * cleanups after test debugging --------- Co-authored-by: Edmund Noble <[email protected]> * MultiNode compaction test (#1742) * rewrite StartedChainweb into GADT syntax * add multi node compaction test * Compaction: log every query * refactor compaction code * STM-based chain selection algo for mining * compact-resume test: move to singletonChainGraph * compaction idempotency test (#1759) * compaction idempotency test * compaction pre-and-post active row test (#1756) * add pre-and-post compaction test * compactTable: log the table name * udpate to latest patience library * benchmark newBlock after compaction (#1774) * [test] compaction tables created after target blockheight are dropped (#1768) * add drop tables test * add latest option for chain compaction * Revert "STM-based chain selection algo for mining" This reverts commit d6cd8e8. * compaction compare pact state tool (#1764) * add pact-diff tool * flush handle in withPerChainFileLogger * remove redundant/bad old Compaction.hs test * respond to some review * finish responding to Austin's review * respond to first round of Edmund's reviews * respond to Edmund's second wave of review * Revert "compact-resume test: move to singletonChainGraph" This reverts commit f4fc0e9. * make compact function take a TargetBlockHeight instead of a BlockHeight * get rid of redundant ITxId type * fix index of getLatestBlockHeight query log * respond to more review * compaction test txlogs (#1780) * txlog compaction test --------- Co-authored-by: Lars Kuhtz <[email protected]> Co-authored-by: Stuart Popejoy <[email protected]> Co-authored-by: Evgenii Akentev <[email protected]> Co-authored-by: Edmund Noble <[email protected]> Co-authored-by: Edmund Noble <[email protected]>

txlog test

73dfba9

chessai marked this pull request as draft November 15, 2023 05:44

chessai added 5 commits November 15, 2023 14:28

more work

5d4d47a

Merge branch 'chessai/compaction' into chessai/compaction-test-txlogs

e470ce6

fix bench compile error

b57758e

txlogsTest

1ba2cc4

finished test

b56a376

chessai requested review from edmundnoble, larskuhtz, thoughtpolice and a user November 16, 2023 04:59

chessai added 4 commits November 15, 2023 23:05

move aeson-pretty to proper place in chainweb-tests build-depends; ad…

c29b6c7

…d version

fix MultiNode for TargetBlockHeight stuff

78954ac

Merge branch 'chessai/compaction' into chessai/compaction-test-txlogs

7ecdfc9

Merge branch 'chessai/compaction' into chessai/compaction-test-txlogs

40df59e

chessai commented Nov 16, 2023

View reviewed changes

chessai marked this pull request as ready for review November 16, 2023 05:26

chessai commented Nov 16, 2023

View reviewed changes

test/Chainweb/Test/Pact/RemotePactTest.hs Outdated Show resolved Hide resolved

chessai commented Nov 16, 2023

View reviewed changes

thoughtpolice approved these changes Nov 16, 2023

View reviewed changes

chessai commented Nov 16, 2023

View reviewed changes

test/Chainweb/Test/Utils.hs Outdated Show resolved Hide resolved

Apply suggestions from self-review

2e95c0b

edmundnoble reviewed Nov 16, 2023

View reviewed changes

remove redundant txlog inequality checking

5de8d31

edmundnoble reviewed Nov 16, 2023

View reviewed changes

respond to review

ebe96f1

edmundnoble reviewed Nov 16, 2023

View reviewed changes

edmundnoble approved these changes Nov 16, 2023

View reviewed changes

chessai merged commit f7ca60d into chessai/compaction Nov 16, 2023
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compaction test txlogs #1780

compaction test txlogs #1780

chessai commented Nov 15, 2023 •

edited

Loading

chessai Nov 16, 2023

edmundnoble Nov 16, 2023

chessai Nov 16, 2023

chessai Nov 16, 2023

chessai Nov 16, 2023

chessai Nov 16, 2023 •

edited

Loading

thoughtpolice left a comment

edmundnoble Nov 16, 2023

chessai Nov 16, 2023

edmundnoble Nov 16, 2023

edmundnoble Nov 16, 2023

chessai Nov 16, 2023

compaction test txlogs #1780

compaction test txlogs #1780

Conversation

chessai commented Nov 15, 2023 • edited Loading

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

chessai Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

thoughtpolice 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

chessai commented Nov 15, 2023 •

edited

Loading

chessai Nov 16, 2023 •

edited

Loading