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

Remove application ticking service #613

Open
bartekn opened this issue Aug 24, 2018 · 0 comments
Open

Remove application ticking service #613

bartekn opened this issue Aug 24, 2018 · 0 comments
Labels

Comments

@bartekn
Copy link
Contributor

bartekn commented Aug 24, 2018

I think we should remove global application ticking service (App.ticks) from Horizon. It causes a lot of obscure synchronization issues between go routines and it makes new code overcomplicated. The root problem is that it's possible that a new function call can be started before the previous one (started by a previous tick) is finished causing issues like overwriting a current state with an old one. If a function depends on a data from another ticking function (ex. App.UpdateLedgerState) the data may be stalled at the data request time.

Example 1 - ingest package

The code in ingest package depends on ledger package state. Both are started by a ticking service. It was possible that a newly started ingest session created a new job based on a stale ledger state. More information, including examples in: #603.

This issue has been finally reproduced and solved in #603 after numerous debugging hours.

Example 2 - fee stats

The code is still in a PR (#586) but we managed to find an issue connected to multiple function calls running at the same time.

The issue made it possible to update a operationfeestats state with a stale data if the function on Tick1 was finished after Tick2 call (actually this issue exists in ledger package now and is hard to fix because operations in UpdateLedgerState are not atomic!).

What instead?

Ticks can be easily replaced by an infinite loop with a sleep if there's no work to do. Simplified go code:

for {
	var coreLatest, historyLatest int32
 	coreQ := core.Q{Session: i.CoreDB}
	err := coreQ.LatestLedger(&coreLatest)
	if err != nil {
		// log.Error(...)
		time.Sleep(time.Second)
		continue
	}

 	historyQ := history.Q{Session: i.HorizonDB}
	err = historyQ.LatestLedger(&historyLatest)
	if err != nil {
		// log.Error(...)
		time.Sleep(time.Second)
		continue
	}

	if historyLatest == coreLatest {
		// no new ledgers
		time.Sleep(time.Second)
		continue
	}

	is.Cursor = NewCursor(historyLatest+1, coreLatest, i)
	is.Run()
}

Because it's not possible that another instance of this function is running at the same time, it's much easier to code and reason about the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant