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

Optimize Relay #108

Merged
merged 23 commits into from
Jan 31, 2024
Merged

Optimize Relay #108

merged 23 commits into from
Jan 31, 2024

Conversation

dongrie
Copy link
Contributor

@dongrie dongrie commented Sep 27, 2023

Optimized execution interval of relay service

@siburu
Copy link
Contributor

siburu commented Oct 3, 2023

@dongrie is this PR ready for review?

@dongrie dongrie marked this pull request as ready for review October 3, 2023 02:00
@dongrie dongrie requested a review from a team as a code owner October 3, 2023 02:00
@dongrie
Copy link
Contributor Author

dongrie commented Oct 3, 2023

@siburu Change to ready for review

core/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
tests/cases/tmmock2tmmock/Makefile Show resolved Hide resolved
@dongrie dongrie requested a review from siburu November 8, 2023 01:38
Copy link
Contributor

@siburu siburu left a comment

Choose a reason for hiding this comment

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

@dongrie I agree with your basic direction. I made some comments on several points.

core/naive-strategy.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
cmd/service.go Outdated Show resolved Hide resolved
@dongrie dongrie requested a review from siburu November 21, 2023 00:25
cmd/service.go Outdated Show resolved Hide resolved
core/naive-strategy.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
core/naive-strategy.go Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Show resolved Hide resolved
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
* Fix start time variable
* Change relay parameter 

Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
@dongrie dongrie requested a review from siburu November 29, 2023 07:10
cmd/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
cmd/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
core/service.go Outdated Show resolved Hide resolved
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
Copy link
Contributor

@siburu siburu left a comment

Choose a reason for hiding this comment

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

@dongrie Sorry for the late review

tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
tests/cases/tmmock2tmmock/scripts/test-service Outdated Show resolved Hide resolved
Signed-off-by: Dongri Jin <[email protected]>
@dongrie
Copy link
Contributor Author

dongrie commented Dec 21, 2023

2593214

Fixed test-service

Use eventHeight


Signed-off-by: Dongri Jin <[email protected]>
Signed-off-by: Dongri Jin <[email protected]>
@dongrie dongrie requested a review from siburu January 10, 2024 01:11
Copy link
Contributor

@siburu siburu left a comment

Choose a reason for hiding this comment

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

@dongrie Sorry for late. I put some comments. They are the last to fix.

} else {
counterpartyCtx = core.NewQueryContext(context.TODO(), counterpartyH.GetHeight())
}

seqs, err := counterparty.QueryUnreceivedPackets(counterpartyCtx, packets.ExtractSequenceList())
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to counterparty query unreceived packets: %w height: %v", err, counterpartyCtx.Height())
Copy link
Contributor

Choose a reason for hiding this comment

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

"failed to query counterparty for unreceived packets: error=%w, height=%v"

Please apply the same changes to similar comments throughout this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed error message
72e4d88

Copy link
Contributor

Choose a reason for hiding this comment

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

@dongrie Please apply the same changes to similar comments throughout this file...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed error messages
72e4d88

core/service.go Outdated Show resolved Hide resolved
tests/cases/tm2tm/scripts/test-service Outdated Show resolved Hide resolved
* Fix check src, dst timestamp
* Fix test case comments


Signed-off-by: Dongri Jin <[email protected]>
@dongrie dongrie requested a review from siburu January 23, 2024 01:42
Signed-off-by: Dongri Jin <[email protected]>
} else {
counterpartyCtx = core.NewQueryContext(context.TODO(), counterpartyH.GetHeight())
}

seqs, err := counterparty.QueryUnreceivedAcknowledgements(counterpartyCtx, packets.ExtractSequenceList())
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to counterparty query unreceived acknowledgements: error=%w height=%v", err, counterpartyCtx.Height())
Copy link
Contributor

Choose a reason for hiding this comment

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

@dongrie failed to query counterparty for unreceived ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3d82c65
Fixed

Signed-off-by: Dongri Jin <[email protected]>
@dongrie dongrie requested a review from siburu January 30, 2024 11:22
} else {
counterpartyCtx = core.NewQueryContext(context.TODO(), counterpartyH.GetHeight())
}

seqs, err := counterparty.QueryUnreceivedAcknowledgements(counterpartyCtx, packets.ExtractSequenceList())
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to query counterparty for unreceived: error=%w height=%v", err, counterpartyCtx.Height())
Copy link
Contributor

Choose a reason for hiding this comment

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

@dongrie unreceived acknowledgements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, Fixed
7a4d0fa

Signed-off-by: Dongri Jin <[email protected]>
@dongrie dongrie requested a review from siburu January 31, 2024 07:12
Copy link
Contributor

@siburu siburu left a comment

Choose a reason for hiding this comment

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

@dongrie Great work, thanks.

@siburu siburu merged commit bf144b0 into hyperledger-labs:main Jan 31, 2024
6 checks passed
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