Skip to content

handle interrupt signal#3

Closed
tzaffi wants to merge 8 commits intoplugin-interfacefrom
graceful-interrupt
Closed

handle interrupt signal#3
tzaffi wants to merge 8 commits intoplugin-interfacefrom
graceful-interrupt

Conversation

@tzaffi
Copy link
Owner

@tzaffi tzaffi commented Jun 9, 2023

Summary

Gracefully handling interrupt signal. The non-gracefulness was discovered during block-generator testing. The following new printouts are being generated (which without the new go-routine are NOT printed).

{"__type":"Conduit","_name":"main","level":"info","msg":"Pipeline received interrupt signal, stopping pipeline. p.pipelineMetadata.NextRound: 14","time":"2023-06-09T14:47:06-05:00"}
{"__type":"importer","_name":"algod","level":"trace","msg":"importer algod.GetBlock() called BlockRaw(14) err: context canceled","time":"2023-06-09T14:47:06-05:00"}
{"__type":"importer","_name":"algod","level":"error","msg":"error getting block for round 14 (attempt 0): context canceled","time":"2023-06-09T14:47:06-05:00"}
{"__type":"importer","_name":"algod","level":"trace","msg":"importer algod.GetBlock() called StatusAfterBlock(13) err: context canceled","time":"2023-06-09T14:47:06-05:00"}
{"__type":"Conduit","_name":"main","level":"error","msg":"GetBlock ctx error: context canceled","time":"2023-06-09T14:47:06-05:00"}
{"__type":"Conduit","_name":"main","level":"info","msg":"Retry number 1 resuming after a 1s retry delay.","time":"2023-06-09T14:47:06-05:00"}
{"__type":"importer","_name":"algod","level":"info","msg":"importer algod.Close() at round 14","time":"2023-06-09T14:47:07-05:00"}
{"__type":"Conduit","_name":"main","level":"info","msg":"Pipeline.Stop(): Importer (algod) closed without error","time":"2023-06-09T14:47:07-05:00"}
{"__type":"importer","_name":"algod","level":"info","msg":"importer algod.Close() at round 14","time":"2023-06-09T14:47:07-05:00"}
{"__type":"Conduit","_name":"main","level":"info","msg":"Pipeline.Stop(): Importer (algod) closed without error","time":"2023-06-09T14:47:07-05:00"}
{"__type":"exporter","_name":"postgresql","level":"info","msg":"exporter postgresql.Close() at round 14","time":"2023-06-09T14:47:07-05:00"}
{"__type":"Conduit","_name":"main","level":"info","msg":"Pipeline.Stop(): Exporter (postgresql) closed without error","time":"2023-06-09T14:47:07-05:00"}
{"__type":"exporter","_name":"postgresql","level":"info","msg":"exporter postgresql.Close() at round 14","time":"2023-06-09T14:47:07-05:00"}

I'm not sure why the closed without error lines are getting repeated for each plugin.

Test Plan

WOMM

if algodImp.cancel != nil {
algodImp.cancel()
}
algodImp.logger.Infof("importer algod.Close() at round %d", algodImp.round)
Copy link

Choose a reason for hiding this comment

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

I don't think we need to save the round in importer. we can print the round in the exporter.

Copy link
Owner Author

@tzaffi tzaffi Jun 9, 2023

Choose a reason for hiding this comment

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

I wanted to introduce the round purely for logging purposes -to know from algod importer's perspective- which round it was on. But this could be overkill. I see several approaches:

  • keep as is
  • revert and get rid of the round
  • rename algodImp.roundalgodImpl.loggingRound to clarify that it's not actually useful for anything except for logging

I don't have a strong opinion here.

}
}
}()

Copy link

Choose a reason for hiding this comment

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

Copy link
Owner Author

Choose a reason for hiding this comment

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

that's better thanks!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Copy link
Owner Author

@tzaffi tzaffi Jun 9, 2023

Choose a reason for hiding this comment

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

But this most recent commit fails tests. First hunch is that this is a real failure.

...nahh. Just a careless copy 🍝 .

Copy link
Owner Author

Choose a reason for hiding this comment

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

@tzaffi tzaffi marked this pull request as ready for review June 12, 2023 14:25
@tzaffi
Copy link
Owner Author

tzaffi commented Jun 12, 2023

Thanks @shiqizng for approving. I'm going to close this PR and re-open it against master once I pull in the latest.

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