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.

Sign up for GitHub

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

Jump to bottom

Collate consecutive heights and sequence numbers shown in logs #2847

Merged
merged 7 commits into from
Nov 10, 2022
Merged

Collate consecutive heights and sequence numbers shown in logs #2847

merged 7 commits into from
Nov 10, 2022

Conversation

romac
Copy link
Member

@romac romac commented Nov 9, 2022 •
edited
Loading

Closes: #2846
Closes: #2142

Description

Collate consecutive sequence numbers and heights in logs

For example, where before we would only show the first 50 sequence numbers:

2022-11-09T09:49:59.911813Z DEBUG ThreadId(386) worker.batch{chain=ibc-1}:supervisor.handle_batch{chain=ibc-1}:supervisor.process_batch{chain=ibc-1}:worker.packet.cmd{src_chain=ibc-1 src_port=transfer src_channel=channel-5 dst_chain=ibc-0}:schedule_packet_clearing{height=Some(Height { revision: 1, height: 26800 })}:relay_pending_packets{height=Some(Height { revision: 1, height: 26799 })}:build_packet_ack_msgs{query_height=1-26799}: sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain (first 50 shown here) dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=[1259, 1260, 1261, 1262, 1263, 1264, 1265, 1266, 1267, 1268, 1269, 1270, 1271, 1272, 1273, 1274, 1275, 1276, 1277, 1278, 1279, 1280, 1281, 1282, 1283, 1284, 1285, 1286, 1287, 1288, 1289, 1290, 1291, 1292, 1293, 1294, 1295, 1296, 1297, 1298, 1299, 1300, 1301, 1302, 1303, 1304, 1305, 1306, 1307, 1308]

We now show all sequences numbers where consecutive ones are collated together:

2022-11-09T09:49:59.911813Z DEBUG ThreadId(386) worker.batch{chain=ibc-1}:supervisor.handle_batch{chain=ibc-1}:supervisor.process_batch{chain=ibc-1}:worker.packet.cmd{src_chain=ibc-1 src_port=transfer src_channel=channel-5 dst_chain=ibc-0}:schedule_packet_clearing{height=Some(Height { revision: 1, height: 26800 })}:relay_pending_packets{height=Some(Height { revision: 1, height: 26799 })}:build_packet_ack_msgs{query_height=1-26799}: sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=1259..=1329, 1359..=1439, 1469..=1500

PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@romac
Remove unused num-traits dependency
0a07128
@romac
Implement Add<u64> and Sub<u64> for Height
1961608
@romac
Implement Add<u64> and Sub<u64> for Sequence
9ef5d1a
@romac
Collate consecutive sequence numbers and heights in logs
318f77b 
For example, whereas before we would only show the first 50 sequence numbers:

    sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain (first 50 shown here) dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=[1259, 1260, 1261, 1262, 1263, 1264, 1265, 1266, 1267, 1268, 1269, 1270, 1271, 1272, 1273, 1274, 1275, 1276, 1277, 1278, 1279, 1280, 1281, 1282, 1283, 1284, 1285, 1286, 1287, 1288, 1289, 1290, 1291, 1292, 1293, 1294, 1295, 1296, 1297, 1298, 1299, 1300, 1301, 1302, 1303, 1304, 1305, 1306, 1307, 1308]

We now show all sequences numbers where consecutive ones are collated together:

    sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=1259..=1329, 1359..=1439, 1469..=1500
@romac
Add changelog entry
f370b49
@romac romac requested a review from adizere November 9, 2022 10:16
@romac romac marked this pull request as ready for review November 9, 2022 10:16
@adizere adizere requested a review from ljoss17 November 9, 2022 18:18
ljoss17
ljoss17 reviewed Nov 10, 2022
View reviewed changes
Copy link
Contributor

@ljoss17 ljoss17 left a comment

Choose a reason for hiding this comment

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

A few comments/questions regarding imports

crates/relayer-cli/src/commands/query/packet/acks.rs Show resolved Hide resolved
crates/relayer-cli/src/commands/query/packet/commitments.rs
@@ -67,8 +60,11 @@ impl QueryPacketCommitmentsCmd {
// cargo run --bin hermes -- query packet commitments --chain ibc-0 --port transfer --channel ibconexfer --height 3
impl Runnable for QueryPacketCommitmentsCmd {
fn run(&self) {
use crate::conclude::json;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

crates/relayer-cli/src/commands/query/packet/pending.rs Show resolved Hide resolved
crates/relayer-cli/src/commands/query/packet/pending_acks.rs Show resolved Hide resolved
crates/relayer-cli/src/commands/query/packet/pending_sends.rs Show resolved Hide resolved
crates/relayer-cli/src/commands/query/packet/util.rs Show resolved Hide resolved
ljoss17
ljoss17 approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@ljoss17 ljoss17 left a comment

Choose a reason for hiding this comment

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

Apart from the comments on the imports, everything seems good!

@romac romac merged commit f1350d6 into master Nov 10, 2022
@romac romac deleted the romac/collate-sequences branch November 10, 2022 11:55
@romac romac mentioned this pull request Nov 10, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljoss17 ljoss17 ljoss17 approved these changes

@adizere adizere Awaiting requested review from adizere

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@romac @ljoss17