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

Fix shutdown persistence issue in Seednode #5388

Merged
merged 1 commit into from Apr 13, 2021
Merged

Fix shutdown persistence issue in Seednode #5388

merged 1 commit into from Apr 13, 2021

Conversation

ghost
Copy link

@ghost ghost commented Apr 3, 2021

Fix issue raised by @Emzy in Keybase ops. Seednode v1.6.0 was not persisting on shutdown.

#5280 introduced a guard in PersistenceManager to not persist at shutdown if a valid app start was not seen.
Initialize PersistenceManager in Seednode app so that the expected persist on shutdown can occur.

@chimp1984 plz check.

[edit] per @chimp1984 suggestion below, moved the init call to AppSetupWithP2P.onBasicServicesInitialized

@chimp1984
Copy link
Contributor

@jmacxx Ah thanks for spotting that!
I think its better to do it in 'AppSetupWithP2P.onBasicServicesInitialized'. There are some other headless apps using those old startup code as well I guess.

@ghost ghost closed this Apr 3, 2021
@ghost ghost reopened this Apr 3, 2021
Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

utACK

@Emzy
Copy link
Contributor

Emzy commented Apr 3, 2021

testedACK
Works on my seednodes

Apr-03 16:57:35.996 [SeedNodeMain] WARN  b.n.p.n.TorNetworkNode: We got called shutDown again and ignore it. 
Apr-03 16:57:35.998 [SeedNodeMain] INFO  b.n.p.n.NetworkNode: Shutdown 16 connections 
Apr-03 16:57:36.044 [SeedNodeMain] INFO  b.c.p.PersistenceManager: Start flushAllDataToDisk 
...
Apr-03 16:57:36.084 [SeedNodeMain] INFO  b.n.p.n.NetworkNode: Shutdown completed with all connections closed 
Apr-03 16:57:36.086 [SeedNodeMain] WARN  b.c.p.PersistenceManager: We got flushAllDataToDisk called again. This can happen in some
 rare cases. We ignore the repeated call. 
Apr-03 16:57:36.090 [SeedNodeMain] INFO  b.c.p.PersistenceManager: flushAllDataToDisk completed 

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK based on #5388 (comment)

@ripcurlx ripcurlx merged commit ca2731e into bisq-network:master Apr 13, 2021
@ripcurlx ripcurlx added this to the v1.6.3 milestone Apr 13, 2021
@ghost ghost mentioned this pull request May 4, 2021
@ghost ghost deleted the fix_seednode_persist branch May 29, 2022 22:46
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