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

Add pg stats #2

Closed
wants to merge 14 commits into from
Closed

Add pg stats #2

wants to merge 14 commits into from

Conversation

preetansh
Copy link
Owner

Welcome to the PR tracker for NoisePage! We're excited that you're interested in improving our system.

Before continuing with opening a PR, please read through the Pull Request Process page on our wiki. PRs that do not follow our guidelines will be immediately closed. In general, you should avoid creating a PR until you are reasonably confident the tests should pass, having tested locally first.

Please choose the appropriate labels on the Github panel and feel free to assign yourself. Additionally, if your PR solves an open issue, please link the issue on the Github panel. However, please DO NOT assign any reviewers to your PR. We will decide who best to assign for a review.

Heading

Please choose an appropriate heading for your PR, relevant to the changes you have made. For example, if your PR addresses the LIMIT clause on IndexScans, an appropriate PR name would be Index Scan Limit.

Description

Please create a description of the issue your PR solves, and how you went about implementing your solution. An example from a PR by @thepinetree follows:

Limit clauses are currently not propagated to the IndexScanPlanNode in the optimizer and as a result, the execution engine can't take advantage of the limit during operation. Instead, this is done in-post, with a LimitPlanNode doing so after the index scan is completed.

This PR adds functionality for the limit value to be pushed down to an index scan, and is used in TPC-C. Limits values will be pushed down to their child LogicalGet via transformation rule and converted to values in the PhysicalIndexScan which are then set in the IndexScanPlanNode. To appropriately act on the Limit value, we also add infrastructure for optional properties for a child to satisfy, which is tracked only in an Optimizer node. The PR also moves the OrderByOrderingType from the optimizer to the catalog as a precursor to further changes to involve the sort direction of columns in creating/scanning an index.

Remaining Tasks

Again, you should only create PR once you are reasonably confident you are near completion. However, if there are some tasks still remaining before the PR is ready to merge, please create a checklist to track active progress. An example from a PR by @thepinetree follows:

📌 TODOs:

  • Stash limit in OptimizerContext for pushdown (INVALID)
  • Move ordering type to catalog
  • Add transformation rule for limit pushdown
  • Add optional property support
  • Fix memory leaks
  • Add GitHub issues for OrderingType investigation, physical prune stage, and TPL break statement

Performance

If your PR has the potential to greatly affect the performance of the system, please address these by benchmarking your changes with respect to master, or profiling the performance. You may do this in one of the following ways:

  1. Inline a table outlining performance results. An example from a PR @gonzalezjo follows:

    Machine Type Terminals Scale Factor Socket Type Transactions / Second
    Bare metal 10 10 UNIX 8346 (+34%)
    Bare metal 10 10 INET 6214
    Bare metal 1 1 UNIX 914 (+37%)
    Bare metal 1 1 INET 668
    VMware 10 10 UNIX 3728 (+24%)
    VMware 10 10 INET 3005
    VMware 1 1 UNIX 289 (+28%)
    VMware 1 1 INET 226
  2. Create a Google Sheets document noting baseline performance and scalability, as is done here in an example from @mbutrovich

  3. Create an SVG of profiling results per the EC2 profiling instructions, as is done here in an example from @thepinetree

Further Work

If your PR unlocked the potential for further improvement, please note them here and create additional issues! Do the same if you discovered bugs in the process of development. An example from a PR by @gonzalezjo follows:

Investigating loopback and TCP overhead

This is probably a dead end, but less of a dead end than libevent/epoll stuff.

I'd like to work on improving our INET socket overhead, but at this point, that might not be doable without being a kernel engineer. Nothing I tried measurably reduced loopback and/or INET overhead, and I tried a lot. Still, I think it's worth digging some more.

One question I have is how much of the speedup comes from avoiding TCP, and how much comes from using a glorified pipe instead of loopback. This would be interesting to know, but I have no idea how I'd measure, and I'm not convinced that I'd be able to make anything useful from the answer.


Here's an empty template to format yourself!

Heading

Description

Remaining tasks

  • Foo
  • Bar
  • Baz

Performance

Further work

Sorry, something went wrong.

gengkev and others added 14 commits April 28, 2020 03:34
Problem: dropping tables doesn't seem to work. Is this our fault?

* Fix more copypaste errors

* More copy paste bugs!!!!!!!!!!!
This has been sitting around on my computer for a while, and I don't
remember much so I can't write a meaningful commit message
Hopefully this is the final boss of copy-paste bugs.
Sanity check added to bwtree_index, so that future variations of this
bug can be caught more easily (when deferred deletes fail, they don't do
so with a stack trace).
Ensures that pg_statistic column is inserted and deleted as appropriate,
and that GetTableStats() successfully uses the statistics stored in that
row.

Is this the right place to put such a test?
* Add pg_statistic hardcoding to recovery_manager
* Fix brace placement in stats_catalog_test
* Reduce copying in DeleteColumnStatistics
@preetansh preetansh closed this Oct 29, 2020
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

3 participants