ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 44 forks source link

Prune/limit storage of rejected & failed swaps #548

Open sangaman opened 6 years ago

sangaman commented 6 years ago

Currently, we savea a SwapDeal from the maker side even when we reject the deal due to a reason like no route or no order availaibility. I think we should change this to only track swaps where the deal has at least been accepted first (whether it ultimately succeeds or not). The goal would be to prevent tracking useless data - in practice I could see it being perfectly normal for swaps to be frequently rejected and it could quickly become a lot of data. This is open for discussion since there may be reasons to want to store rejected swaps that I am not aware of.

offerm commented 6 years ago

This is open for discussion since there may be reasons to want to store rejected swaps that I am not aware of.

Exactly. there may be reasons to want to store rejected swaps. This is why we should store them.

I could see it being perfectly normal for swaps to be frequently rejected

What can be a good reason for this?

As an exchange/admin, I would like to know about these failures, analyze them and reduce future failures.

Also, this sounds like an optimization and we should defer it to a later stage when we better understand the usage patterns and be sure that we need to optimize.

sangaman commented 6 years ago

I'm not talking about failures due to an error per se but rather swaps that were never attempted in the first place. In an environment where there is a lot of trading going on, I imagine it could be fairly common to get multiple swap requests for the same order in a short period of time, where we'd have to reject all but one. Although I believe with #495 swaps rejected due to an order on hold are not stored ,so we wouldn't need to change anything on that front.

The other case that's currently being stored that I'm thinking we might not want to store is when a swap fails due to not being able to communicate with local LND instances, either they are disabled or a getinfo call fails.

In these cases the errors would still be logged, just potentially not stored to the database.

offerm commented 6 years ago

I understand you. Still, I think we should not remove this "potential noise" until it becomes "real noise". Let's see how things develop and keep in mind that we can always do this. Doing it now does not give real value. My 2 cents

kilrau commented 5 years ago

I think we can store more or less everything for now and at some point prune failed swaps after a certain time/amount. Implementing this is what this issue is about.

engwarrior commented 4 years ago

Pls provide estimate for this task.