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: No need to compute point hash if there is only one shard #20118

Merged
merged 4 commits into from
Nov 30, 2020

Conversation

StoneYunZhao
Copy link
Contributor

@StoneYunZhao StoneYunZhao commented Nov 20, 2020

No need to compute point hash if there is only one shard contained in shard group. We can save a lot of computing resources because of two reason:

  1. In the OSS version of the single-node architecture, a shard group must only contain one shard.
  2. Each point in the write request needs to call the ShardFor method.
  • CHANGELOG.md updated
  • Rebased/mergable
  • Tests pass
  • Sign CLA (if not already signed)

@danxmoran
Copy link
Contributor

Hi @StoneYunZhao thanks for the contribution! While we review, could you push an update to the CHANGELOG.md file with a reference to this PR?

@StoneYunZhao
Copy link
Contributor Author

StoneYunZhao commented Nov 21, 2020

Hi @StoneYunZhao thanks for the contribution! While we review, could you push an update to the CHANGELOG.md file with a reference to this PR?

@danxmoran Done.

And I have another question. Should I request merge to branch 1.8 or branch master. The write request in version 2.x will reference code under v1 directory eventually.

@danxmoran
Copy link
Contributor

Should I request merge to branch 1.8 or branch master. The write request in version 2.x will reference code under v1 directory eventually.

We'll eventually want to merge the change to:

  • 1.8
  • master-1.x
  • (if applicable to V2) master
  • (if applicable to V2) 2.0

For a patch this small, it doesn't matter as much where we start. The back/forward-port PRs typically follow from the original review & merge to minimize the extra work needed if changes are requested.

Copy link
Contributor

@danxmoran danxmoran left a comment

Choose a reason for hiding this comment

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

The optimization makes sense to me, I've suggested one change to the log entry (plus a shout-out for you!)

I'm curious about your motivations for opening the PR. Did you see slowness in your deployment that you traced back to the shard lookup? Or did you notice an opportunity for improvement when reading through the code?

CHANGELOG.md Outdated Show resolved Hide resolved
@StoneYunZhao
Copy link
Contributor Author

StoneYunZhao commented Nov 24, 2020

The optimization makes sense to me, I've suggested one change to the log entry (plus a shout-out for you!)

I'm curious about your motivations for opening the PR. Did you see slowness in your deployment that you traced back to the shard lookup? Or did you notice an opportunity for improvement when reading through the code?

My work in the company includes functional enhancements and performance optimizations for InfluxDB. Some enhancements and optimizations are specific to the company's scenarios, and some are general. For general optimization and enhancement, we hope to give back to the community. I found the optimization when reading through the code.

Co-authored-by: Daniel Moran <[email protected]>
@danxmoran
Copy link
Contributor

Thanks for the contribution @StoneYunZhao, I'll handle forward-porting the patch to all the relevant branches.

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.

3 participants