Projekt

Obecné

Profil

Chyba #1129

uzavřený

Zobrazení převodů člena

Přidáno uživatelem Michal Kliment před více než 4 roky(ů). Aktualizováno před více než 4 roky(ů).

Stav:
Uzavřený
Priorita:
Vysoká
Přiřazeno:
Kategorie:
Finance
Cílová verze:
Začátek:
2019-10-30
Uzavřít do:
% Hotovo:

100%

Odhadovaná doba:

Popis

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


Související úkoly 1 (0 otevřených1 uzavřený)

související s Požadavek #959: Mezisoučet u seznamu převodu členaUzavřenýDavid Raška2015-02-17

Akce

Aktualizováno uživatelem Michal Kliment před více než 4 roky(ů)

  • související s Požadavek #959: Mezisoučet u seznamu převodu člena přidán

Aktualizováno uživatelem Jakub Juračka před více než 4 roky(ů)

  • Přiřazeno změněn z Jakub Juračka na Filip Miškařík

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

  • Stav změněn z Nový na Odeslaný
  • % Hotovo změněn z 0 na 90

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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).

Aktualizováno uživatelem Jakub Juračka před více než 4 roky(ů)

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? :)

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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.

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

Michale, co si o tom myslíš?

Aktualizováno uživatelem Michal Kliment před více než 4 roky(ů)

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...

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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íš.

Aktualizováno uživatelem Michal Kliment před více než 4 roky(ů)

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í.

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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ů).

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

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.

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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.

Aktualizováno uživatelem Michal Kliment před více než 4 roky(ů)

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ů.

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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.

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

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

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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).

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

Máš na mysli tuto podmínku?

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

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

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';
}

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

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í.

Aktualizováno uživatelem Filip Miškařík před více než 4 roky(ů)

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

Aktualizováno uživatelem Ondřej Fibich před více než 4 roky(ů)

  • Stav změněn z Odeslaný na Uzavřený
  • % Hotovo změněn z 90 na 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.

Také k dispozici: Atom PDF