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

Migrate data to new transactions table #207

Merged
merged 37 commits into from
Jun 14, 2024

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Jun 10, 2024

Also, create data migration infrastructure so that future migrations can be easily added.

@2opremio 2opremio requested a review from tamirms June 10, 2024 19:35
@2opremio 2opremio force-pushed the transactions-db-migration branch from c1011cd to daf4ca8 Compare June 10, 2024 19:46
@2opremio 2opremio force-pushed the transactions-db-migration branch 3 times, most recently from 123755d to 331e630 Compare June 11, 2024 03:21
@2opremio 2opremio marked this pull request as ready for review June 11, 2024 03:23
@2opremio 2opremio requested a review from psheth9 June 11, 2024 03:23
@2opremio 2opremio force-pushed the transactions-db-migration branch from 331e630 to 7b40026 Compare June 11, 2024 03:45
@2opremio 2opremio force-pushed the transactions-db-migration branch from 7b40026 to 8f3abca Compare June 11, 2024 04:08
@2opremio 2opremio requested a review from aditya1702 June 11, 2024 04:14
@tamirms
Copy link
Contributor

tamirms commented Jun 11, 2024

@2opremio overall looks good! Could you add some tests?

@2opremio
Copy link
Contributor Author

@tamirms Sure thing!

@2opremio 2opremio force-pushed the transactions-db-migration branch from ee26069 to 9e581ab Compare June 13, 2024 13:49
@2opremio 2opremio force-pushed the transactions-db-migration branch from 966ac43 to ce7194d Compare June 13, 2024 15:11
@2opremio 2opremio force-pushed the transactions-db-migration branch from 0c7fc5c to 3f4ca3f Compare June 13, 2024 17:56
@2opremio
Copy link
Contributor Author

2opremio commented Jun 13, 2024

Uhm, I am starting to think it's a problem with the JSONRPC client.

If I print the listening ports right after the error, I see that soroban RPC is indeed listening on port 8000

    integration.go:265: RPC still unhealthy Post "http://localhost:8000": dial tcp [::1]:8000: connect: connection refused 
[...]
Who is listening on RPC port?
COMMAND    PID   USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
test.test 9885 runner   17u  IPv4 115645      0t0  TCP localhost:8000 (LISTEN)

I think that the client may not retry after the first attempt if it cannot connect :S

@2opremio 2opremio force-pushed the transactions-db-migration branch from bca6f2c to 5c4c87e Compare June 13, 2024 19:48
@2opremio
Copy link
Contributor Author

@tamirms This is finally ready to go.

The new migration test required rewriting most of the integration-test infra, so the PR ended up getting quite big (sorry about that).

As a result the integration test infra is way cleaner.

BTW, the spurious test failures I was fighting were due to creachadair/jrpc2#118

Copy link
Contributor

@tamirms tamirms left a comment

Choose a reason for hiding this comment

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

great job debugging creachadair/jrpc2#118 , this looks great!

@2opremio 2opremio merged commit f7580ad into stellar:main Jun 14, 2024
22 checks passed
@2opremio 2opremio deleted the transactions-db-migration branch June 14, 2024 10:48
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.

3 participants