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

feat: Make logs easy to reason about #28

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

khorolets
Copy link
Member

@khorolets khorolets commented Feb 23, 2022

Closes #11

Logs will be printed every 10 seconds like in nearcore

Example:

INFO near_lake: # 2508 | Blocks processing: 0| Blocks done: 2509. Bps 250.90 b/s  | 0.00 s to catch up the tip

# 2508 - last processed block by near_lake
Blocks processing: 0 - number of blocks currently being processed
Blocks done: 2509 - total number of processed blocks
Bps 250.90 b/s - blocks per second (during last 10 sec)
0s to catch up the tip - seconds to catch up with the tip of the network

Originally in the issue @frol said

List of the block heights that are processed (if there are too many of them [let's say 5], print 4 oldest ones and 1 the most recent)

But I don't think it'd be helpful to print a bunch of blocks height, I think the number of the currently in-progress blocks should be enough. However, we can change it later.

@khorolets khorolets added this to the 1.0 milestone Feb 23, 2022
@khorolets khorolets requested a review from telezhnaya February 23, 2022 09:00
src/main.rs Outdated
drop(stats_lock);

let block_processing_speed: f64 =
(stats_copy.done_count as f64 - prev_done_count as f64) / interval_secs as f64;

Choose a reason for hiding this comment

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

I'd prefer to add braces here and in all places where you do math + casts.
It could divide by interval_secs without changing the type, and then cast the result to f64.
I don't remember the parsing order, it's better not to rely on this

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope I get it right, could you have a look again?

Choose a reason for hiding this comment

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

You got it right, now it's better!

src/main.rs Outdated
#[derive(Debug, Clone)]
struct Stats {
pub processing: std::collections::BTreeSet<u64>,
pub done_count: u64,

Choose a reason for hiding this comment

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

Let's rename this variable?
blocks_processed_count or anything else

src/main.rs Outdated

const INDEXER: &str = "near_lake";

#[derive(Debug, Clone)]
struct Stats {
pub processing: std::collections::BTreeSet<u64>,

Choose a reason for hiding this comment

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

Let's rename this variable? It's not intuitive

src/main.rs Outdated
@@ -15,16 +15,16 @@ const INDEXER: &str = "near_lake";

#[derive(Debug, Clone)]
struct Stats {
pub processing: std::collections::BTreeSet<u64>,
pub done_count: u64,
pub blocks_processing: std::collections::BTreeSet<u64>,

Choose a reason for hiding this comment

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

block_heights_processing?

@@ -15,6 +15,7 @@ aws-sdk-s3 = "0.6.0"
aws-smithy-http = "0.36.0"
clap = { version = "3.0.0-beta.5", features = ["color", "derive", "env"] }
futures = "0.3.5"
humantime = "2.1.0"

Choose a reason for hiding this comment

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

I thought about our random coffee at first 😅

src/main.rs Outdated
@@ -111,14 +111,14 @@ async fn lake_logger(
drop(stats_lock);

let block_processing_speed: f64 =
(stats_copy.done_count as f64 - prev_done_count as f64) / interval_secs as f64;
((stats_copy.blocks_processed_count - prev_done_count) as f64) / (interval_secs as f64);

Choose a reason for hiding this comment

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

Let's also rename prev_done_count

@khorolets khorolets merged commit 05324f4 into main Feb 23, 2022
@khorolets khorolets deleted the refactor/improve-logs-and-errors branch February 23, 2022 21:33
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.

Make info logs easy to reason about
2 participants