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.

Sign up for GitHub

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

Jump to bottom

The function name OperationDriver::ApplyAsync is confused #384

Closed
lorinlee opened this issue Jul 16, 2018 · 2 comments
Closed

The function name OperationDriver::ApplyAsync is confused #384

lorinlee opened this issue Jul 16, 2018 · 2 comments
Assignees
@mbautin @spolitov
Labels
kind/question This is a question

Comments

@lorinlee
Copy link

lorinlee commented Jul 16, 2018 •
edited
Loading

When reading the source code of yugabyte-db, I find that the function name OperationDriver::ApplyAsync is confused, and the comments of this function is outdated. It said that "ApplyAsync() submits ApplyTask() to the apply_pool_", but in fact it is not true. This comment and the invoking means the procedure is synchronous. Furthermore, I think the apply_pool_ in OperationDriver and even TabletPeer should be removed, because I cannot find any usage of apply_pool_ in them.
@kmuthukk kmuthukk added the kind/question This is a question label Jul 16, 2018
@kmuthukk
Copy link
Collaborator

Hi @lorinlee:

You may be right that the comments might have fallen out of sync :). But I will defer to @mbautin or @spolitov to confirm.

@spolitov
Copy link
Contributor

Hi @lorinlee,

You are right, the comment and function name were not updated after the behaviour of this function was changed during refactoring.
Good point to address this issue during one of upcoming refactorings.

yugabyte-ci pushed a commit that referenced this issue Aug 22, 2018
@mbautin
#384 Rename ApplyAsync to ApplyOperation and get rid of the unused ap…
41b9a6e 
…ply thread pool

Summary:
To paraphrase @lorinlee on #384: The function name
OperationDriver::ApplyAsync is confusing, and the comments of this function is outdated. It says
that "ApplyAsync() submits ApplyTask() to the apply_pool_", but in fact it is not true. The
invocation of this function is synchronous. Also, apply_pool_ in OperationDriver and even TabletPeer
should be removed because they are not being used.

Test Plan: Jenkins

Reviewers: sergei, bogdan, hector

Reviewed By: hector

Subscribers: bharat, kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D5292
mbautin added a commit that referenced this issue Jul 11, 2019
@mbautin
This commit provides additional functionality that is logically relat…
037b536 
…ed to the

earlier commit 41b9a6e

Original commit message:

#384 Rename ApplyAsync to ApplyOperation and get rid of the unused apply thread pool

Summary:
To paraphrase @lorinlee on #384: The function name
OperationDriver::ApplyAsync is confusing, and the comments of this function is outdated. It says
that "ApplyAsync() submits ApplyTask() to the apply_pool_", but in fact it is not true. The
invocation of this function is synchronous. Also, apply_pool_ in OperationDriver and even TabletPeer
should be removed because they are not being used.

Test Plan: Jenkins

Reviewers: sergei, bogdan, hector

Reviewed By: hector

Subscribers: bharat, kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D5292
mbautin added a commit to mbautin/yugabyte-db that referenced this issue Jul 16, 2019
@mbautin
yugabyte#384 Rename ApplyAsync to ApplyOperation and get rid of the u…
76b77df 
…nused apply thread pool

Summary:
To paraphrase @lorinlee on yugabyte#384: The function name
OperationDriver::ApplyAsync is confusing, and the comments of this function is outdated. It says
that "ApplyAsync() submits ApplyTask() to the apply_pool_", but in fact it is not true. The
invocation of this function is synchronous. Also, apply_pool_ in OperationDriver and even TabletPeer
should be removed because they are not being used.

Test Plan: Jenkins

Reviewers: sergei, bogdan, hector

Reviewed By: hector

Subscribers: bharat, kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D5292

Note:
This commit provides additional functionality that is logically related to
the earlier commit yugabyte@41b9a6e
and supersedes the commit yugabyte@037b536
@ryan-ally ryan-ally mentioned this issue Dec 21, 2023
Open
jasonyb pushed a commit that referenced this issue Jun 11, 2024
@codeforall
PG-607: Allow histogram to track queries in sub-ms time brackets (#384)
fe23d31 
* PG-607: Allow histogram to track queries in sub-ms time brackets

Updated the GUC configuration and the relevant histogram functionality
to track queries in lower cardinality than ms. This is done by saving
the GUC values for histogram min and max values in real (double) type.

All test cases except for the 030 tap test are passing. The test case
needs an update.

* Fixing regression issues for v12 and below because of histogram changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mbautin mbautin

@spolitov spolitov

Labels
kind/question This is a question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mbautin @spolitov @kmuthukk @lorinlee