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

Rocks db window service #1888

Merged
merged 11 commits into from
Nov 25, 2018
Merged

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Nov 22, 2018

Problem

Current implementation of window service doesn't use new db_window module

Summary of Changes

  1. Integrate RocksDb into window service
  2. Fix associated tests

Fixes #

@carllin carllin requested a review from rob-solana November 22, 2018 11:53
@carllin carllin force-pushed the RocksDbWindowService branch from aceeefb to 5840165 Compare November 22, 2018 12:09
@carllin carllin closed this Nov 22, 2018
@carllin carllin reopened this Nov 22, 2018
@carllin carllin closed this Nov 22, 2018
@carllin carllin reopened this Nov 23, 2018
@carllin carllin force-pushed the RocksDbWindowService branch 9 times, most recently from 38ce50c to cfd82e4 Compare November 23, 2018 08:15
let (replicator, leader_info) = Replicator::new(
db_ledger,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be better if you passed the ledger_path as the argument, did the opening in src/fullnode.rs. This way we can move to sled without changing this code.

Copy link
Contributor Author

@carllin carllin Nov 24, 2018

Choose a reason for hiding this comment

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

I think this comment was meant for bin/fullnode.rs? I made the suggested change there :)

Copy link
Contributor

Choose a reason for hiding this comment

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

is the change not appropriate here, as well?

Copy link
Contributor Author

@carllin carllin Nov 25, 2018

Choose a reason for hiding this comment

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

The replicator doesn't open a fullnode, but I can move the open logic to src/replicator.rs to parallel the change in src/fullnode.rs

extern crate solana_metrics;

use clap::{App, Arg};
use rocksdb::{Options, DB};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to encapsulate these types so moving to sled is easier

@@ -273,6 +274,8 @@ pub fn process_blob(
let is_coding = blob.read().unwrap().is_coding();

// Check if the blob is in the range of our known leaders. If not, we return.
// TODO: Need to update slot in broadcast, otherwise this check will fail with
Copy link
Contributor

Choose a reason for hiding this comment

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

how about a github issue instead?

use solana::client::mk_client;
use solana::cluster_info::{Node, FULLNODE_PORT_RANGE};
use solana::db_ledger::{write_entries_to_ledger, DB_LEDGER_DIRECTORY};
Copy link
Contributor

Choose a reason for hiding this comment

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

how come everybody has to pass in DB_LEDGER_DIRECTORY? could db_ledger do that for them, instead?

@carllin carllin force-pushed the RocksDbWindowService branch 5 times, most recently from 9bfbef6 to 2a1fb73 Compare November 24, 2018 08:05
@carllin carllin force-pushed the RocksDbWindowService branch 3 times, most recently from 78a94fe to 6116063 Compare November 24, 2018 12:45
@carllin carllin force-pushed the RocksDbWindowService branch from 2075726 to c9ece71 Compare November 25, 2018 00:43
@carllin carllin force-pushed the RocksDbWindowService branch 5 times, most recently from 9718256 to e792b9c Compare November 25, 2018 01:59
@carllin carllin force-pushed the RocksDbWindowService branch from e792b9c to 0153f12 Compare November 25, 2018 02:49
@carllin carllin merged commit 57a384d into solana-labs:master Nov 25, 2018
@carllin carllin mentioned this pull request Nov 26, 2018
vkomenda pushed a commit to vkomenda/solana that referenced this pull request Aug 29, 2021
web3validator pushed a commit to web3validator/solana that referenced this pull request Jun 27, 2024
* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function
tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024
tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024
…olana-labs#1888) (solana-labs#1900)

* Refactor and additional metrics for cost tracking (solana-labs#1888)

* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function

(cherry picked from commit c3fadac)

* rename cost_tracker.account_data_size to better describe its purpose is to tracker per-block new account allocation

---------

Co-authored-by: Tao Zhu <82401714+tao-stones@users.noreply.github.com>
Co-authored-by: Tao Zhu <tao@solana.com>
tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024
tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024
tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024
* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function
andreisilviudragnea pushed a commit to andreisilviudragnea/solana that referenced this pull request Jul 2, 2024
* Refactor and additional metrics for cost tracking (solana-labs#1888)

* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function

* remove histogram for loaded accounts size due to performance impact
yihau pushed a commit to yihau/solana that referenced this pull request Jul 18, 2024
…port of solana-labs#1888) (solana-labs#1900) (solana-labs#1937)

Revert "v2.0: Refactor and additional metrics for cost tracking (backport of solana-labs#1888) (solana-labs#1900)"

This reverts commit 0aef62e.
yihau pushed a commit to yihau/solana that referenced this pull request Jul 18, 2024
…abs#1975)

* Refactor cost tracking (solana-labs#1954)

* Refactor and additional metrics for cost tracking (solana-labs#1888)

* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function

* remove histogram for loaded accounts size due to performance impact

(cherry picked from commit f8630a3)

* rename cost_tracker.account_data_size to better describe its purpose is to tracker per-block new account allocation

---------

Co-authored-by: Tao Zhu <82401714+tao-stones@users.noreply.github.com>
Co-authored-by: Tao Zhu <tao@solana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants