Skip to content

[fpmsyncd] support pipeline to flush with a timer#3241

Merged
prsunny merged 4 commits intosonic-net:masterfrom
eddieruan-alibaba:flushwithtimer
Nov 27, 2024
Merged

[fpmsyncd] support pipeline to flush with a timer#3241
prsunny merged 4 commits intosonic-net:masterfrom
eddieruan-alibaba:flushwithtimer

Conversation

@a114j0y
Copy link
Contributor

@a114j0y a114j0y commented Jul 22, 2024

What I did

  • change the pipeline size for RouteSync
  • modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
    • the timer is simulated by setting the timeout for select function

Why I did it

  • to batch the data that fpmsyncd flush to APP_DB

How I verified it
measure the performance with PerformanceTimer GitHub issue/pull request detail

Copy link
Contributor

@siqbal1986 siqbal1986 left a comment

Choose a reason for hiding this comment

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

Is there a test covering this change. how can we see if this is working as intended.
Also the build is failing. Can you please look into it.

@a114j0y
Copy link
Contributor Author

a114j0y commented Oct 7, 2024

Is there a test covering this change. how can we see if this is working as intended. Also the build is failing. Can you please look into it.

Its compilation needs this pr to be released

@a114j0y a114j0y force-pushed the flushwithtimer branch 2 times, most recently from 45568ef to 5af28d2 Compare October 22, 2024 21:56
@a114j0y a114j0y marked this pull request as draft October 30, 2024 23:44
@a114j0y a114j0y force-pushed the flushwithtimer branch 3 times, most recently from 30707f8 to dcc16ad Compare November 18, 2024 21:55
@a114j0y a114j0y marked this pull request as ready for review November 19, 2024 19:42
int idle = pipeline.getIdleTime();

// flush right away
if (remaining < SMALL_TRAFFIC || idle >= gFlushTimeout || idle <= 0 || scheduled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why flush with small traffic?
If there are lots of data, the pipeline will not flush until next timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This optimization aims to reduce pipeline flushes in the case of scaled traffic.

Hence for SMALL_TRAFFIC, do immediate flush to maintain the logic before optimization. So that it would not affect the performance when you want to dispatch several critical routes.

But since we skip several flushes, we need to set a timeout to make sure the routes would eventually get flushed, that's when idle >= gFlushTimeout

idle <=0 happens when the system clock is not steady, but since we use steady_clock it shouldn't happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

#Closed

Suggest add this info to code comments.

@a114j0y
Copy link
Contributor Author

a114j0y commented Nov 21, 2024

This PR also depends on merging PR#954

prsunny
prsunny previously approved these changes Nov 22, 2024
@prsunny
Copy link
Collaborator

prsunny commented Nov 25, 2024

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit 98303af into sonic-net:master Nov 27, 2024
@a114j0y a114j0y deleted the flushwithtimer branch November 27, 2024 01:24
bradh352 pushed a commit to bradh352/sonic-swss that referenced this pull request Dec 4, 2024
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
bradh352 pushed a commit to bradh352/sonic-swss that referenced this pull request Dec 4, 2024
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
divyachandralekha pushed a commit to divyachandralekha/sonic-swss that referenced this pull request Dec 12, 2024
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
divyachandralekha pushed a commit to divyachandralekha/sonic-swss that referenced this pull request Dec 12, 2024
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
stepanblyschak pushed a commit to stepanblyschak/sonic-swss that referenced this pull request Jan 27, 2025
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
shiraez pushed a commit to Marvell-switching/sonic-swss that referenced this pull request Feb 17, 2025
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function
baorliu pushed a commit to baorliu/sonic-swss that referenced this pull request Feb 23, 2026
What I did
change the pipeline size for RouteSync
modify the fpmsyncd flush behavior so that it could delay the flush with an expiring timer
the timer is simulated by setting the timeout for select function

Signed-off-by: Baorong Liu <[email protected]>
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.

5 participants