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

Ikc 71 first page problem #151

Merged
merged 6 commits into from
Jul 8, 2021
Merged

Ikc 71 first page problem #151

merged 6 commits into from
Jul 8, 2021

Conversation

ynleborg
Copy link
Contributor

@ynleborg ynleborg commented Jul 7, 2021

No description provided.

@ynleborg ynleborg changed the base branch from ikc-66-event-tracking-backend to master July 7, 2021 11:23
@ynleborg ynleborg requested a review from mmergo-cd July 7, 2021 11:26
@@ -36,7 +36,7 @@ import {ServersService} from '../servers.service';
export class TopicPaginationComponent {
@Input() paging: Page;
@Input() topicName: string;
pageLimits = [10, 20, 50, 100];
pageLimits = [1, 5, 10, 20, 50, 100, 500, 1000];
Copy link
Collaborator

Choose a reason for hiding this comment

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

1? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

scenariusz w którym masz np 128 partycji

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aaa, bo to jest per partycja teraz. Ma to sens

@@ -85,7 +85,7 @@ export class TopicBackendService implements TopicService {
initPaging(): void {
const paging = new Page();
paging.pageNumber = 1;
paging.size = 20;
paging.size = 10;
Copy link
Collaborator

Choose a reason for hiding this comment

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

to też na pewno chcemy na stałe zmienić?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moze lepiej powinno być uzależnione od liczby partycji? Jak jest jedna albo mało może być 20, ale jak jest np 128 to wręcz powinno się domyślnie ustawić na 1 albo 5

Copy link
Collaborator

Choose a reason for hiding this comment

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

W sumie, to by było spoko. Ale może na teraz faktycznie zostawmy 10, i wrócimy ewentualnie do tematu

}
}
log.debug("TCM20 poll completed records.size={}", messages.size());
IntStream
Copy link
Collaborator

Choose a reason for hiding this comment

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

Czy to nie spowoduje, że absolutnie zawsze i wszędzie zrobimy polla 5 razy? Ta stara pętla była taka se, ale miała zdaje się ten plus, że jeśli nazbieraliśmy wystarczająco wiadomości, to ją wcześniej przerywaliśmy.
A może to jest celowo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mam pomysl na optymalizację, który polega na tym, że przerywam polla gdy zajdzie jedno z dwóch: mamy wystarczającą liczbę eventów zeby zwrócić do frontu albo dwa kolejne polle nic nie zwróciły

Copy link
Collaborator

Choose a reason for hiding this comment

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

brzmi jak plan ;)

@@ -11,7 +11,8 @@
[headerHeight]="38"
[scrollbarH]="false"
[scrollbarV]="false"
[columnMode]="'force'">
[columnMode]="'force'"
(activate)="navigateToTrack($event)">
Copy link
Collaborator

Choose a reason for hiding this comment

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

zacny pomysł, żeby to dać prosto na tabeli nagłówków!

Copy link
Contributor Author

@ynleborg ynleborg Jul 7, 2021

Choose a reason for hiding this comment

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

true, tutaj jeszcze będzie przekazany kontekst, ale to w osobnym zadaniu

@ynleborg ynleborg merged commit 5c073ae into master Jul 8, 2021
@ynleborg ynleborg deleted the ikc-71-first-page-problem branch July 8, 2021 07:52
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.

None yet

2 participants