Skip to content

[common] enable redispipeline to only publish after flush #895

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

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

a114j0y
Copy link
Contributor

@a114j0y a114j0y commented Jul 22, 2024

What I did

  • optimize redispipeline flush performance by remove unnecessary publish commands
  • add a new parameterbool flushPub in producerstatetable constructor function
    • to enable/disable batch publish feature
    • default value of m_flushPub is false, so no impact on existing codes
    • optimization is effective only explicitly set this option
  • remove individual publish command from the producerstatetable APIs' lua scripts
  • add a publish command when the pipeline flushes [if m_flushPub is true]

Why I did it

  • save TCP traffic and increase fpmsyncd efficiency

It's a feature included in BGP Loading Optimization HLD #1521 GitHub issue/pull request detail

@a114j0y a114j0y force-pushed the flushpub branch 2 times, most recently from 81ee88f to 3ef9655 Compare August 25, 2024 23:31
@a114j0y a114j0y force-pushed the flushpub branch 6 times, most recently from 5c0a375 to d033f37 Compare August 30, 2024 21:57
@a114j0y a114j0y force-pushed the flushpub branch 6 times, most recently from 65cba5f to 1d27f30 Compare September 24, 2024 00:31
@a114j0y a114j0y requested a review from liuh-80 October 4, 2024 21:46
@a114j0y a114j0y force-pushed the flushpub branch 4 times, most recently from d2cb95f to a3bd808 Compare October 7, 2024 23:40
@liuh-80 liuh-80 requested a review from qiluo-msft October 8, 2024 01:15
liuh-80
liuh-80 previously approved these changes Oct 8, 2024
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@a114j0y a114j0y force-pushed the flushpub branch 4 times, most recently from 1891c61 to bb7155a Compare October 31, 2024 23:57
@a114j0y a114j0y force-pushed the flushpub branch 2 times, most recently from 641fd83 to 2d8b19b Compare November 1, 2024 22:07
@eddieruan-alibaba
Copy link

@liuh-80 do you have further comments?

@a114j0y a114j0y force-pushed the flushpub branch 4 times, most recently from 34083c4 to fb3a9c5 Compare November 6, 2024 22:43
liuh-80
liuh-80 previously approved these changes Nov 8, 2024
@a114j0y
Copy link
Contributor Author

a114j0y commented Nov 8, 2024

just rebased on upstream/master

@yuezhoujk
Copy link

/azp run Azure.sonic-swss-common

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yuezhoujk
Copy link

/azp run Azure.sonic-swss-common

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft merged commit 901f3b4 into sonic-net:master Nov 18, 2024
17 checks passed
VladimirKuk pushed a commit to Marvell-switching/sonic-swss-common that referenced this pull request Nov 20, 2024
)

What I did

optimize redispipeline flush performance by remove unnecessary publish commands
add a new parameterbool flushPub in producerstatetable constructor function
to enable/disable batch publish feature
default value of m_flushPub is false, so no impact on existing codes
optimization is effective only explicitly set this option
remove individual publish command from the producerstatetable APIs' lua scripts
add a publish command when the pipeline flushes [if m_flushPub is true]
Why I did it

save TCP traffic and increase fpmsyncd efficiency
It's a feature included in BGP Loading Optimization HLD #1521
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.

6 participants