Project

General

Profile

Chyba #1129

Zobrazení převodů člena

Added by Michal Kliment 3 months ago. Updated about 1 month ago.

Status:
Uzavřený
Priority:
Vysoká
Category:
Finance
Target version:
Start date:
10/30/2019
Due date:
% Done:

100%

Estimated time:

Description

Zobrazení převodů člena je velmi pomalé a navíc ve špatném pořadí. Zřejmě souvisí s #959


Related issues

Related to Požadavek #959: Mezisoučet u seznamu převodu členaUzavřený02/17/2015

Actions

History

#1

Updated by Michal Kliment 3 months ago

#2

Updated by Jakub Juračka 3 months ago

  • Assignee changed from Jakub Juračka to Filip Miškařík
#3

Updated by Filip Miškařík 3 months ago

  • Status changed from Nový to Odeslaný
  • % Done changed from 0 to 90
#4

Updated by Ondřej Fibich 2 months ago

Patch se mi nelíbí. Kvůli takové blbosti se tam musí dělat takové hnusy..

Osobně bych navrhoval funkcionalitu předělat:
- Sloupec "Mezisoučet" nezobrazovat pokud je aktivní filtr.
- Díky tomu je možné mezisoučet počítat ze současného balance účtu (tj. ze současnosti počítám minulost). Tj. bez dalších SQL příkazů.

Zvažte prosím zda by to takto nebylo přijatelné. Osobně si myslím, že na filtrovaných datech to není natolik zajímavá informace.

P.S. Filipe, prosím o kontrolu nastavení odsazení kódu v NetBeans. Stále to není optimální, v commitech jsou odsazení s 8 mezerami (má b 1 TAB). Také prosím o nastavení NetBeans tak, aby namísto CRLF zapisoval pouze LF (tedy Unix zakončení řádků namísto Windows).

#5

Updated by Jakub Juračka 2 months ago

Pokud se k tomu můžu taky vyjádřit, tak bych to nenazval hnusem v porovnání s tím jak to bylo řešeno před updatem. Pokud si dobře vzpomínám tak pro 500 transakcí se provádělo v callbacku 500 SQL dotazů, při kterých se propočítával celý mezisoučet znova a znova, takže načítání trvalo někdy i přes 20 sekund...

Já si naopak myslím, že když už tam mezisoučet je, tak není na škodu ho mít dostupný i při filtrování (třeba podle data), aby člověk věděl stav účtu v určité době. Tudíž za tuto cenu provádět jen 2 SQL dotazy a pak mezisoučty propočítat v PHPku je podle mě asi nejefektivnější řešení.

K tomu propočítávání ze současného balance:
- Není mi jasná jedna věc. Když mám 500 transakcí a mám třeba 50 transakcí na stránku, tak když si zobrazím třeba stranu 3, tak se to bude propočítávat jak? :)

#6

Updated by Ondřej Fibich 2 months ago

Ahoj,

On to ten update moc nevylepšil. Pro účet s mnoha převody to dokáže klidně shodit DB. Vybírá se tam až 100 000 záznamů.
Už ani neřeším, že to celé nefunguje, pokud má účet 100 001 převodů..

Jediné možné optimální řešení s podporou filtrování je cachování mezisoučtu v DB. Podobně jako balance účtu nepočítáme online. To je ale kvůli takové okrajové funkcionalitě kanón na vrabce. Ono totiž je jedna věc to nacachovat, ale druhá je tu cache udržovat (přepočtení příspěvků a podobné funkcionality to značně komplikují).

Proto navrhuji jednoduché funkční řešení, které má ale omezení ve filtru. Myslím si ale, že omezení není nijak závažné.

Stránkování lze vyřešit pro stranu 3 SQL dotazem účtu ID 4:

SELECT (
    (SELECT SUM(t.amount) FROM transfers t WHERE t.destination_id = 4 ORDER BY ... LIMIT 0 150) - 
    (SELECT SUM(t.amount) FROM transfers t WHERE t.origin_id = 4 ORDER BY ... LIMIT 0 150)
) AS start_balance

Tedy dopočítám si počáteční balance pro záznamy tabluky.

#7

Updated by Ondřej Fibich 2 months ago

Michale, co si o tom myslíš?

#8

Updated by Michal Kliment 2 months ago

Můj názor je, že žádný nemám :-) Každopádně je to potřeba vyřešit co nejdříve, protože například načtení převodů tohoto člena trvá 46 vteřin...

#9

Updated by Ondřej Fibich about 2 months ago

Michal Kliment napsal:

Můj názor je, že žádný nemám :-) Každopádně je to potřeba vyřešit co nejdříve, protože například načtení převodů tohoto člena trvá 46 vteřin...

Můj patch je ready: https://gitlab.slavicin.unart.cz/freenetis/freenetis/merge_requests/92
Mergni přes Gitlab pokud souhlasíš.

#10

Updated by Michal Kliment about 2 months ago

Stále to obsahuje původní chybu v řazení podle sloupce id - správně má být
podle sloupce datetime. Některé převody byly do DB vloženy v jiném pořadí
než vznikly ve skutečnosti (například vstupní příspěvky).

Při testování se navíc ukázalo, že uživatelé systému (rozuměj Jovák ☺ ) se
přejí zobrazení sloupce i při použití filtru, případně při změně řazení.

#11

Updated by Ondřej Fibich about 2 months ago

Michal Kliment napsal:

Stále to obsahuje původní chybu v řazení podle sloupce id - správně má být
podle sloupce datetime. Některé převody byly do DB vloženy v jiném pořadí
než vznikly ve skutečnosti (například vstupní příspěvky).

Při testování se navíc ukázalo, že uživatelé systému (rozuměj Jovák ☺ ) se
přejí zobrazení sloupce i při použití filtru, případně při změně řazení.

Řazení podle datetime bych zvládl udělat. Filtrování, ale optimálně a bez před-počítání hodnoty mezisoučtu v DB, neumím.
Před-počítání by byla velká zátěž na údržbu. Kód pro update by musel být na více místech. Jedině vše spojené s převody nacpat do jedné service, která by si to hlídala, aby to nějaký controller nepokazil.. To je ale kopec práce.

Ještě to pls zvažte. Osobně nechápu, k čemu to filtrování může být zrovna tam tak užitečné/důležité..
Patch od Filipa je špatný na tom já trvám, ale nijak zásadně to prosazovat nebudu. Pro zobrazení kreditních podúčtů členů bude fungovat. U účtů sdružení to bude horší než současný stav (vzhledem k počtu převodů, které jsou u účtů).

#12

Updated by Filip Miškařík about 2 months ago

Upravil jsem to tak, aby to nepadalo při 100 001 převodech. Jovák trvá na tom, aby mezisoučty byly viditelné i při zapnutém filtru. Chtěl bych se zeptat co je myšleno tím, že při zobrazení převodů sdružení to bude horší než současný stav.

#13

Updated by Ondřej Fibich about 2 months ago

Filip Miškařík napsal:

Upravil jsem to tak, aby to nepadalo při 100 001 převodech. Jovák trvá na tom, aby mezisoučty byly viditelné i při zapnutém filtru. Chtěl bych se zeptat co je myšleno tím, že při zobrazení převodů sdružení to bude horší než současný stav.

Například Provozní účet má aktuálně 429 696 převodů. Tvé nové řešení je bude stahovat všechny, což povede k tomu, že dotaz skončí po 30s na timeoutu nebo to shodí celou DB. Proto se mi Tvé řešení nelíbí. Jak jsem psal, staré je sice horší, ale tohle u velkého počtu převodů není taky žádné terno. Pokud by tohle řešení mělo být opravdu použito, pak by možná šlo zakázat sloupec (a neprovádět SQL) pro účty s více než 1000 převodů. Jak ale znám Jováka, tak se mu to líbit nebude..

Osobně bych v tomhle Jováka obešel. Doteď to tam neměl vůbec a najednou je z toho nezbytnost :-).

Michale, rozsuď to pls.

#14

Updated by Michal Kliment about 2 months ago

Tak ten mezisoučet nechejme pouze pro kreditní účty? Účet s největším
počtem převodů má 656 převodů, což ně úrovni SQL je úplné nic.

Aktuálně jsou ve Freenetisu daleko větší výkonnostní problémy - například
aktualizace stavů z monitoringu trvá minimálně 2-3 minuty, protože se tam
dělá SQL dotaz pro každé zařízení (pro 1000 zařízení je to 1000 SQL
updatů). Další problém je práce s e-maily, kde jsou uchovávany e-maily
staré několik let a tabulka má teď už 2485376 záznamů.

#15

Updated by Ondřej Fibich about 2 months ago

Souhlas, jen pro kreditní účty to bude fungovat v pohodě. Já právě nevěděl, jestli to nechcete i jinde..
Filipe, nachystáš prosím úpravu patche?

U monitoringu by to chtělo použít jinou DB a ne MySQL/MariaDB, která nezvládá časté zápisy. Vhodná je TimescaleDB. Například Zabbix na ni aktuálně taky přechází.

E-maily vyřešíme v #1137.

#16

Updated by Filip Miškařík about 2 months ago

Nový patch je připravený. Zkus se na to prosím podívat, jestli je to v pořádku.

#17

Updated by Ondřej Fibich about 2 months ago

Filip Miškařík napsal:

Nový patch je připravený. Zkus se na to prosím podívat, jestli je to v pořádku.

Funkčně vypadá OK, až na vyhozenou podmínku kontrolující $order_by. Byla špatně, ale musí tam být kvůli SQL injection.

Přeformátování celého controlleru ale nebyl dobrý nápad. Prosím o opravu (revert změny) a aplikaci správného formátování pouze na dotčené modifikované části.

Celkové přeformátování mi dělá velké problémy, pokud mám backportovat (cherry pickovat) změnu z master větve do developu (vytváří to pak složité konflikty).

#18

Updated by Filip Miškařík about 1 month ago

Máš na mysli tuto podmínku?

if (strtolower($order_by_direction) != 'desc')
{
    $order_by_direction = 'desc';
}

#19

Updated by Ondřej Fibich about 1 month ago

Filip Miškařík napsal:

Máš na mysli tuto podmínku?
[...]

Ano, správně má být:

if (strtolower($order_by_direction) != 'desc')
{
    $order_by_direction = 'asc';
}
#20

Updated by Filip Miškařík about 1 month ago

Nešlo by to s tím formátováním udělat tak, že by jsi mi vyexportoval nastavení tvého NetBeans a já bych to importoval do svého? Pak už by fungovalo automatické formátování ne? A už bychom pak nenaráželi na špatné formátování.

#21

Updated by Filip Miškařík about 1 month ago

Opravil jsem to formátování, tak snad to bude OK.

#22

Updated by Ondřej Fibich about 1 month ago

  • Status changed from Odeslaný to Uzavřený
  • % Done changed from 90 to 100

Formátování vyžadovalo ještě úpravu. Nalezl jsem ale ještě chybu při filtrování, pokud se vyfiltroval převod "uprostřed", pak se subtotal počítal špatně. Chyba byla v tom, že pro převody k výpočtu se nesmí aplikovat filtr. Podívej se prosím do mého commitu.

Also available in: Atom PDF