feat: add v1 schema #528

iand · 2021-05-26T10:25:03Z

Consolidate all v0 migrations into a new v1 schema with some improvements. Visor supports both v0 and v1 schemas when writing directly to database so this won't block feature updates while we migrate to the newer schema. CSV output uses the latest schema.

Incorporates the following PRs:

restore: use numeric type to represent numbers #361 - use numeric columns
schema: add integer now function to each hypertable #378 - add integer now to hypertables
fix(migration): multisig tx param field is nullable #470 - multisig tx param field is nullable (fixes actorstatesmultisig reporting null value in params column #459)
Fix #484: Add Height to IDAddress and ChainEconomics #526 - Add Height to IDAddress and ChainEconomics (fixes Add height to tables without height #484)

Also:

Adds fil reserve disbursed to chain economics and changes column types to numeric (fixes Chaineconomics: add FilReserveDisbursed #462)
Flattens migrations for v1 (fixes schema: Flatten migrations #225)

NOTE: I did look at changing the schema name to visor (for #218) but it turned out to be a much larger and more invasive change than I wanted, especially since I am trying to give us a period of compatibility to allow us to migrate between v0 and v1 of the schema. Go-pg has no explicit support for different schemas (workaround here) and there are some rough edges when using some of the timescale functions such as set_integer_now_func (which needs the schema embedded in the name of the table passed to it)

Summary of Schema Changes

Added a new function current_height which returns the expected current epoch. All hypertables have been set to use current_height as their "now" function.

The following tables are no longer hypertables due to low rates of growth:

chain_powers
chain_rewards
miner_infos
miner_sector_deals
multisig_approvals

The following tables have been converted to hypertables:

derived_gas_outputs: weekly chunks

The following tables have had their hypertable chunking adjusted:

messages: weekly, was daily
receipts: weekly, was daily
miner_sector_posts: daily, was weekly

The following tables have additional columns:

chain_economics: height, fil_reserve_disbursed
id_addresses: height
derived_gas_outputs: actor_family

The following tables have had columns changed to use numeric data type:

chain_economics: circulating_fil, vested_fil, mined_fil, burnt_fil, locked_fil
chain_powers: total_raw_bytes_power, total_raw_bytes_committed, total_qa_bytes_power, total_qa_bytes_committed, total_pledge_collateral, qa_smoothed_position_estimate, qa_smoothed_velocity_estimate
chain_rewards: cum_sum_baseline, cum_sum_realized, effective_baseline_power, new_baseline_power, new_reward_smoothed_position_estimate, new_reward_smoothed_velocity_estimate, total_mined_reward, new_reward numeric
derived_gas_outputs: value, gas_fee_cap, gas_premium, parent_base_fee, base_fee_burn, over_estimation_burn, miner_penalty, miner_tip numeric, refund
message_gas_economy: gas_limit_total, gas_limit_unique_total, base_fee, value, gas_fee_cap, gas_premium, fee_debt
miner_locked_funds: locked_funds, initial_pledge, pre_commit_deposits
miner_pre_commit_infos: pre_commit_deposit, deal_weight, verified_deal_weight
miner_sector_infos: deal_weight, verified_deal_weight, initial_pledge, expected_day_reward, expected_storage_pledge
parsed_messages: value
power_actor_claims: raw_byte_power, quality_adj_power

New tables:

internal_messages
internal_parsed_messages

codecov-commenter · 2021-05-26T10:31:04Z

Codecov Report

Merging #528 (254274c) into master (8b0faf2) will decrease coverage by 1.5%.
The diff coverage is 14.6%.

@@           Coverage Diff            @@
##           master    #528     +/-   ##
========================================
- Coverage    42.1%   40.6%   -1.6%     
========================================
  Files          40      40             
  Lines        3100    3237    +137     
========================================
+ Hits         1308    1315      +7     
- Misses       1583    1713    +130     
  Partials      209     209

iand · 2021-05-28T13:02:18Z

Since this is a wide reaching PR I requested review from all 3 of you (@hsanjuan, @frrist, @placer14 ) to bring it to your attention, not necessarily to review it in its entirety.

I expect we will tag visor as v0.7.0 after this is merged.

frrist

still reviewing, adding a comment while I remember.

frrist · 2021-05-27T23:27:17Z

model/actors/reward/chainreward.go

+	NewRewardSmoothedPositionEstimate string `pg:"type:numeric,notnull"`
+	NewRewardSmoothedVelocityEstimate string `pg:"type:numeric,notnull"`
+	TotalMinedReward                  string `pg:"type:numeric,notnull"`
+	NewReward                         string `pg:"type:numeric,use_zero"`


This should be notnull instead of use_zero, since its a string

hsanjuan · 2021-05-31T17:47:48Z

I did look at changing the schema name to visor (for #218) but it turned out to be a much larger and more invasive change than I wanted

Can we allow a configurable single schema name to use? When we load the backfill it should preferably go into the visor schema on a pristine DB, so compatibility should not matter. We can keep backfilling on that DB until Lily can take over.

iand · 2021-06-01T14:23:22Z

I did look at changing the schema name to visor (for #218) but it turned out to be a much larger and more invasive change than I wanted

Can we allow a configurable single schema name to use? When we load the backfill it should preferably go into the visor schema on a pristine DB, so compatibility should not matter. We can keep backfilling on that DB until Lily can take over.

See this commit d6273b1

frrist · 2021-06-01T20:15:18Z

model/actors/miner/feedebt.go

@@ -42,5 +77,16 @@ func (ml MinerFeeDebtList) Persist(ctx context.Context, s model.StorageBatch, ve
 	if len(ml) == 0 {
 		return nil
 	}
+
+	if version.Major != 1 {
+		// Support older versions, but in a non-optimal way


Is something like this worth logging? (Here and elsewhere in the persist methods)

It could be a really noisy log messages since it would fire on every write. I'll look at issuing a warning when visor starts

frrist · 2021-06-01T23:39:51Z

storage/migrate.go

-	if target.Major != dbVersion.Major {
+	// Version 0.0 indicates that the database has not been initialised with a schema yet
+	if target.Major != dbVersion.Major &&
+		!(dbVersion.Major == 0 && dbVersion.Patch == 0) {


nit: if this check is performed elsewhere dbVersion.IsInitalized() would be a bit cleaner.

I rewrote this area to check explicitly whether the version tables are initialized or not

iand · 2021-06-03T14:24:41Z

Added actor_family column to derived_gas_outputs which is just the last segment of the actor name after the version but having it in the table allows us to index it and remove various wildcard hacks that are used in queries and monitoring.

iand · 2021-06-03T15:44:06Z

Added internal message tables as per #482 (comment)

frrist · 2021-06-03T22:36:21Z

model/messages/internal.go

+	From      string   `pg:",notnull"`
+	To        string   `pg:",notnull"`
+	Value     string   `pg:"type:numeric,notnull"`
+	Method    uint64   `pg:",use_zero"`


should be of type string based on comment here: #482 (comment)

good catch, will fix

I've copy-pasted these structs (I fixed this bit) into my internal_message branch: #533. Are you alright with #533 landing as a follow-on to this?

Yep, let's do it as a follow-up

* feat: enable models to be version aware * Use a structured version * Move schema migrations to new versioned package * Add base SQL for schemas and update README * Fix some spelling errors caught by lint * Appease the lint god * Incorporate pr review feedback Create v1 schema and adjust hypertable parameters Use numeric type to represent numbers add integer now function to each hypertable multisig tx param field is nullable Add Height to IDAddress and ChainEconomics Fix ups to ensure schema v1 matches models Some lint fixes Ensure older schema version can be installed in new database Add add FilReserveDisbursed for chain economics Support backwards compatibility with v0 models Fix genesis test Support custom postgresql schema names Explicitly check whether database has initialized version tables Fix ChainReward.NewReward to be notnull Convert value column in parsed_messages to numeric Add actor_family colum to derived_gas_outputs Add internal_messages and internal_parsed_messages models Make InternalParsedMessage.Method a string to match schema Remove extension comment Only set base schema search path if non-default Fix typo Add version number to initial version tables Fix template typo Revert adding version number to initial version tables Remove extension creation Remove owner statements from v1 schema Use fixed statediff Use merged version in statediff repo tweak logger name

iand force-pushed the iand/schema-v1-2 branch from ccb9363 to 0111eba Compare May 26, 2021 10:25

iand linked an issue May 26, 2021 that may be closed by this pull request

actorstatesmultisig reporting null value in params column #459

Closed

iand added this to the Schema-related issues before full backfill milestone May 26, 2021

iand requested review from frrist, hsanjuan and placer14 May 28, 2021 13:00

frrist reviewed May 28, 2021

View reviewed changes

frrist approved these changes Jun 2, 2021

View reviewed changes

frrist reviewed Jun 3, 2021

View reviewed changes

iand force-pushed the iand/schema-v1-2 branch 3 times, most recently from 6452539 to 32306a9 Compare June 10, 2021 13:12

iand force-pushed the iand/schema-v1-2 branch from 32306a9 to 28961a0 Compare June 10, 2021 13:12

iand self-assigned this Jun 10, 2021

iand merged commit ddacfc7 into master Jun 10, 2021

iand deleted the iand/schema-v1-2 branch June 10, 2021 17:44

frrist mentioned this pull request Jul 27, 2021

Move visor tables to visor schema #218

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add v1 schema #528

feat: add v1 schema #528

iand commented May 26, 2021 •

edited

Loading

codecov-commenter commented May 26, 2021 •

edited

Loading

iand commented May 28, 2021

frrist left a comment

frrist May 27, 2021 •

edited

Loading

iand Jun 3, 2021

hsanjuan commented May 31, 2021

iand commented Jun 1, 2021

frrist Jun 1, 2021

iand Jun 3, 2021

frrist Jun 1, 2021

iand Jun 3, 2021

iand commented Jun 3, 2021

iand commented Jun 3, 2021

frrist Jun 3, 2021

iand Jun 4, 2021

frrist Jun 4, 2021

iand Jun 4, 2021

feat: add v1 schema #528

feat: add v1 schema #528

Conversation

iand commented May 26, 2021 • edited Loading

Summary of Schema Changes

codecov-commenter commented May 26, 2021 • edited Loading

Codecov Report

iand commented May 28, 2021

frrist left a comment

Choose a reason for hiding this comment

frrist May 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsanjuan commented May 31, 2021

iand commented Jun 1, 2021

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

iand commented Jun 3, 2021

iand commented Jun 3, 2021

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

iand commented May 26, 2021 •

edited

Loading

codecov-commenter commented May 26, 2021 •

edited

Loading

frrist May 27, 2021 •

edited

Loading