Move permanent flags to config file #1609

sgillespie · 2024-01-19T19:40:51Z

Description

Fixes #1576. Move configuration flags that can't be changed (disable-ledger, prune-tx-out, disable-shelley, and so on) and add them to the main configuration file.

Checklist

Commit sequence broadly makes sense
Commits have useful messages
New tests are added if needed and existing tests are updated
Any changes are noted in the changelog
Code is formatted with fourmolu on version 0.10.1.0 (which can be run with scripts/fourmolize.sh)
Self-reviewed the diff

Migrations

The pr causes a breaking change of type a,b or c
If there is a breaking change, the pr includes a database migration and/or a fix process for old values, so that upgrade is possible
Resyncing and running the migrations provided will result in the same database semantically

If there is a breaking change, especially a big one, please add a justification here. Please elaborate
more what the migration achieves, what it cannot achieve or why a migration is not possible.

Cmdv · 2024-02-15T15:43:27Z

cardano-chain-gen/test/testfiles/config-conway/test-db-sync-config-bootstrap.json

+  "insert_options": {
+    "tx_out": "bootstrap"
+  }
+}


unsure if you are already doing this but could these not be a single file with updated value, here is a simple example:

cardano-db-sync/cardano-chain-gen/test/Test/Cardano/Db/Mock/Unit/Conway/CommandLineArg/MigrateConsumedPruneTxOut.hs

Lines 60 to 65 in 045330d

where

-- an example of how we will pass our custom configs overwriting the init file

mkSynNodeConfig :: IO SyncNodeConfig

mkSynNodeConfig = do

initConfigFile <- mkSyncNodeConfig conwayConfigDir

pure $ initConfigFile {dncEnableLogging = True}

but for this scenarios it would be:

where mkSynNodeConfig :: IO SyncNodeConfig mkSynNodeConfig = do -- have 1 init file that is read and parsed initConfigFile <- mkSyncNodeConfig conwayConfigDir -- update the results with required values for the specific test in above tests -- it's "tx_out": "bootstrap" so that is reflected as so pure $ initConfigFile { dncInsertConfig = initInsertConfig { spcTxOut = TxOutBootstrap } } -- have an init for specific set of tests or all tests if required initInsertConfig :: SyncInsertConfig initInsertConfig = SyncInsertConfig { spcTxOut = TxOutEnable , spcLedger = LedgerEnable , spcShelley = ShelleyDisable ... }

I've written a couple new tests in this style, but the majority I've kept unchanged

cardano-db-sync/src/Cardano/DbSync/Api.hs

Cmdv · 2024-02-22T14:11:33Z

doc/configuration.md

-Disables fetching pool offchain metadata.
+**Only UTxO**
+
+This is the equivalent of using `--dont-use-ledger` `--disable-shelley`,


might want to mention that this refers to command line flags we previously had.

This equates to the following deprecated flags: --dont-use-ledger --disable-shelley...

I believe the docs should basically be finished now.

Rather than from CLI args

The majority of flags that affect the insert configuration are now part of the primary configuration file for db-sync

kderme · 2024-02-24T08:27:08Z

I had this error, which I think used to be a warning

cardano-db-sync -- --config ../cardano-configs/sanchonet/db-sync-config-new.json --socket-path ../db-paths/sancho.socket --state-dir ../ledger-state/sancho-4-0-0-config --schema-dir schema 
cardano-db-sync: Error: If not using --state-dir then make sure to have ledger disabled. For more details view https://github.com/IntersectMBO/cardano-db-sync/blob/master/doc/syncing-and-rollbacks.md#ledger-state

This seems to hapen with "ledger" : "disable" or with some preset values.

kderme · 2024-02-24T00:09:03Z

cardano-db-sync/src/Cardano/DbSync/Config/Types.hs

  , enpSnEveryFollowing :: !Word64
  , enpSnEveryLagging :: !Word64
  , enpMaybeRollback :: !(Maybe SlotNo)
+  , enpForceTxIn :: !Bool -- TODO[sgillespie]


This can probably be removed, since it's moved to TxOutConfig

kderme · 2024-02-24T00:11:17Z

cardano-db-sync/src/Cardano/DbSync/Config/Types.hs

+      Just "disable_all" -> pure DisableAllInsertOptions
+      Just other -> fail $ "unexpected preset: " <> show other
+
+instance ToJSON SyncInsertConfig where


Are these ToJSON necessary or used anywhere?

These ToJSON instances are used in Hedgehog roundtrip tests

kderme · 2024-02-25T21:17:27Z

cardano-db-sync/src/Cardano/DbSync/Api.hs

+disableAllInsertOptions =
+  SyncInsertOptions
+    { sioTxOut = TxOutDisable
+    , sioLedger = LedgerDisable


For onlyGovInsertOptions the ledger should be enabled by default.
Ideally the ledger should remain an option even with preset insert configuration. For this pr it would be enough to change the only gov to have LedgerEnable.

kderme · 2024-02-26T12:09:45Z

cardano-db-sync/app/cardano-db-sync.hs

+
+      void $ case (maybeLedgerStateDir, ledgerCfg) of
+        -- It's an error to disable ledger and specify the state dir
+        (Just _, LedgerDisable) -> error stateDirErrorMsg


I think this case should just give a warning

* force_tx_in * preset

Update docs to use the config file, rather than CLI args.

* Validate state-dir/ledger config * Lookup in/out from config * Disable rewards in some presets

kderme

Great work!

sgillespie force-pushed the feature/flags->config branch 2 times, most recently from 77a4d86 to 57dd896 Compare January 26, 2024 17:35

sgillespie force-pushed the feature/flags->config branch 8 times, most recently from 52ee6bb to 5bfab2e Compare February 14, 2024 21:40

Cmdv reviewed Feb 15, 2024

View reviewed changes

sgillespie force-pushed the feature/flags->config branch 3 times, most recently from b09f746 to 834c062 Compare February 20, 2024 16:46

Cmdv mentioned this pull request Feb 21, 2024

Proposal for cardano-db-sync: Consider Migration to ClickHouse for Enhanced Performance and Scalability #1586

Open

sgillespie force-pushed the feature/flags->config branch from 46d89d3 to ff65b89 Compare February 22, 2024 12:59

Cmdv reviewed Feb 22, 2024

View reviewed changes

sgillespie force-pushed the feature/flags->config branch 13 times, most recently from 4ea34de to 1c5b202 Compare February 22, 2024 21:46

sgillespie force-pushed the feature/flags->config branch 4 times, most recently from d1c61df to 49a5723 Compare February 23, 2024 19:34

sgillespie added 5 commits February 23, 2024 14:35

feature(cardano-db-sync): Add a new InsertConfig type

185082e

test(cardano-chain-gen): Allow tests to override config file name

83801c5

test(cardano-db-sync): Extract Hedgehog generators into new module

8ce0972

feature(cardano-db-sync): Look up insert settings from config

edf01bc

Rather than from CLI args

refactor(cardano-db-sync): Deprecate configuration flags

40311af

The majority of flags that affect the insert configuration are now part of the primary configuration file for db-sync

sgillespie force-pushed the feature/flags->config branch from 49a5723 to 722d5a8 Compare February 23, 2024 19:37

sgillespie marked this pull request as ready for review February 23, 2024 20:22

kderme requested changes Feb 24, 2024

View reviewed changes

kderme reviewed Feb 25, 2024

View reviewed changes

kderme reviewed Feb 26, 2024

View reviewed changes

sgillespie added 3 commits February 26, 2024 10:45

feature(cardano-db-sync): Add new fields to insert options

34040db

* force_tx_in * preset

doc: Update configuration documentation

f73c986

Update docs to use the config file, rather than CLI args.

fix(cardano-chain-gen): Fix insert options issues

9bc14a3

* Validate state-dir/ledger config * Lookup in/out from config * Disable rewards in some presets

sgillespie force-pushed the feature/flags->config branch from 8cafde8 to 9bc14a3 Compare February 26, 2024 15:46

kderme approved these changes Feb 27, 2024

View reviewed changes

kderme merged commit 59f8eed into IntersectMBO:master Feb 27, 2024
7 of 10 checks passed

sgillespie deleted the feature/flags->config branch February 27, 2024 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move permanent flags to config file #1609

Move permanent flags to config file #1609

sgillespie commented Jan 19, 2024 •

edited

Loading

Cmdv Feb 15, 2024

sgillespie Feb 23, 2024

Cmdv Feb 22, 2024 •

edited

Loading

sgillespie Feb 22, 2024

kderme commented Feb 24, 2024

kderme Feb 24, 2024

kderme Feb 24, 2024

sgillespie Feb 26, 2024 •

edited

Loading

kderme Feb 25, 2024 •

edited

Loading

kderme Feb 26, 2024

kderme left a comment

	where
	-- an example of how we will pass our custom configs overwriting the init file
	mkSynNodeConfig :: IO SyncNodeConfig
	mkSynNodeConfig = do
	initConfigFile <- mkSyncNodeConfig conwayConfigDir
	pure $ initConfigFile {dncEnableLogging = True}

Move permanent flags to config file #1609

Move permanent flags to config file #1609

Conversation

sgillespie commented Jan 19, 2024 • edited Loading

Description

Checklist

Migrations

Cmdv Feb 15, 2024

Choose a reason for hiding this comment

sgillespie Feb 23, 2024

Choose a reason for hiding this comment

Cmdv Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

sgillespie Feb 22, 2024

Choose a reason for hiding this comment

kderme commented Feb 24, 2024

kderme Feb 24, 2024

Choose a reason for hiding this comment

kderme Feb 24, 2024

Choose a reason for hiding this comment

sgillespie Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

kderme Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

kderme Feb 26, 2024

Choose a reason for hiding this comment

kderme left a comment

Choose a reason for hiding this comment

sgillespie commented Jan 19, 2024 •

edited

Loading

Cmdv Feb 22, 2024 •

edited

Loading

sgillespie Feb 26, 2024 •

edited

Loading

kderme Feb 25, 2024 •

edited

Loading