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

[FEAT] Dedupe index actions #9340

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open

Conversation

tommydeboer
Copy link
Collaborator

@tommydeboer tommydeboer commented Oct 13, 2021

TODO

  • testjes fixen
  • testjes schrijven
  • api test / IT fixen
  • index jobs + groups opruimen, canceled moet ook opgeruimd
  • index status SKIPPED toevoegen?
  • heeft het groepje überhaupt nog zin? -> nee
  • checken: bij bootstrap de pending jobs opnieuw schedulen
  • overvoeren ES: minder threads? -> was geheugenprobleem: dev-env meer geheugen gegeven
  • met de bezem er door (bijv. huppeldepupMetadata.IndexAction)
  • andere log levels
  • verwijder ElasticsearchService.refreshIndex() en onderliggend? Alleen nog gebruikt in IT. Te veel werk voor nu.

Checklist

  • Functionality works & meets specifications
  • Code reviewed
  • Code unit/integration/system tested
  • Migration step added in case of breaking change
  • User documentation updated
  • (If you have changed REST API interface) view-swagger.ftl updated
  • Test plan template updated
  • Clean commits
  • Added Feature/Fix to release notes

@fdlk
Copy link
Contributor

fdlk commented Oct 13, 2021

  • Transactions kunnen een nummertje krijgen ipv een ID. Maar wat heb je eraan? :)

  • Als een job start kan hij zien welke IndexActions hij gaat opruimen

  • Dat kan in een MREF naar de acties, net zoals nu, maar dan gegroepeerd op wat bij elkaar hoort ipv transaction ID

  • Bij het aanmaken van de acties ben je zelf nog niet gecommit en dus kun je geen dubbelen vinden

  • Na de commit, bij scheduleIndexJob, kan dat wel

  • Wat hoort er bij elkaar in 1 job?

  • Waarom eigenlijk jobs en geen workers?

  • database is vooral handig om te zorgen dat het na een reboot weer opstart

  • kijkt er iemand ooit in die job logs in de database of kan dat net zo goed naar de console? (we denken van wel)

Idee:
ThreadPoolExecutor met een eigen queue van runnable IndexActions. Kun je dan nog cancellen?

Na de commit, ipv scheduleIndexJob, werk je de queue bij en schrijf je de nieuwe acties naar de database.
Hoe bijwerken:

  • als hij er al op staat, gewoon laten staan en niks toevoegen (of achteraan zetten juist?)
  • als je hele tabel toevoegt, de losse rijen weggooien uit de queue

Zolang we van ClientFacade af blijven kan dit gewoon naar de master

Eerst even niks meer met job framework doen, en dan misschien later wel weer gebruiken

@sonarcloud
Copy link

sonarcloud bot commented Nov 8, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@tommydeboer tommydeboer marked this pull request as ready for review November 9, 2021 12:15
@sonarcloud
Copy link

sonarcloud bot commented Jan 26, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

65.4% 65.4% Coverage
0.0% 0.0% Duplication

@tommydeboer
Copy link
Collaborator Author

@tommydeboer tommydeboer force-pushed the feat/dedupe-index-actions branch 6 times, most recently from a9307e0 to 51d9856 Compare April 5, 2022 08:37
@sonarcloud
Copy link

sonarcloud bot commented Apr 15, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

65.1% 65.1% Coverage
0.0% 0.0% Duplication

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