{"push_id":1006221962,"size":47,"distinct_size":1,"ref":"refs/heads/spencerkimball/fix-proto","head":"ff086a38cec45e560ddae128bab30fae567adce4","before":"68d23c334cdf817dc57142dfe4989fdd1dda3719","commits":[{"sha":"c80009ce02f018b504a7d84f73489ed3d60022ff","author":{"email":"885ea01d97daf7784d4a0ae7f66c69a3686faeb6@gmail.com","name":"Nathan VanBenschoten"},"message":"storage: Trim overlapping timestamp intervals in TimestampCache\n\nFixes #4616 and closes #4700. This is an alternative to #4699.\n\nPreviously, we only removed timestamp intervals from the `TimestampCache`\nduring addition when a newer timestamp interval itself fully superseded\nthe earlier timestamp interval. This failed to take into account that only\nthe most recent timestamps for any given interval was needed to be\nretained by the cache (we only need to keep the upper edge). With this new\nassumption, newer timestamp intervals can be allowed to trim/split/or\ndelete any intervals that come before them and that they overlap. By\nperforming this reduction in size to any intervals that are overlapped,\nwe increase the chance that future timestamps will fully supersede past\ntimestamps, permitting their removal. It also seems to increase the\nchance that past intervals can be replaced in-place with future intervals\nby simply replacing their cache value. This allows us to more precisely\ndetermine when a timestamp interval is still contributing to the cache's\nmaximum timestamp for some segment of the keyspace, and if it's not, remove\nit on the spot instead of waiting for the `MinTSCacheWindow` (currently set\nto 10 seconds) before evicting the interval.\n\nThis missing optimization during `TimestampCache` insertion meant that the\n`TimestampCache` grew to extreme sizes, often accounting for over **300 MB**\nper-node, which meant that it accounted for about **88%** of all memory\nusage for Cockroach nodes. With this new optimization now being diligent\nabout removing timestamp intervals from the timestamp cache as soon as\nthey can be removed, we are able to reduce the upper bound for the per-node\n`TimestampCache` size to about **3 MB**, a **100 x** reduction in size.\nThis means that we are able to reduce the total Cockroach node memory usage\nfrom around **360 MB** to around **60 MB**!\n\nAnother key here is that when we adjust intervals in the `TimestampCache`,\nwe **NEED** to make sure the ordering in the FIFO queue stays consistent\nwith interval timestamps. For instance, if we don't move the interval back\nin the queue in the value replacement case, intervals will get to a point\nwhere their eviction is blocked on the newly replaced interval. This can\nlead to memory growth of upwards of 40 MB!\n\nBecause this optimization does more work during `TimestampCache` insertion, it\nis expected that it will hurt performance slightly. That said, if benchmarks are\nrun as is, they will lead to misleading/incorrect results. This is because the\n`MinTSCacheWindow` is currently set to 10 seconds, meaning that pre-optimization\nbenchmarks never had to perform costly timestamp cache deletions during eviction\nbecause they always finished before this time. Keep in mind while looking at them\nthat after the 10 seconds, this `MinTSCacheWindow` eviction was previously still\nhappening synchronously during timestamp insertion, it was just pushing the cost\nof deletion 10 seconds into the future. The only real discrepancies in\nwork comes from the need to now occasionally split intervals, which\nresults in an additional insertion and an additional deletion from the\ncache.\n\nComparing this PR to #4699:\n\nCPU performance\n- Less computation needed on insert because fewer intervals will\n  overlap since they will be trimmed as we go.\n- This difference can be seen in each change's respective benchmarks.\n\nMemory performance\n- Average TSCache size is **3 MB** here vs. **4 MB** there\n- #4699 will always have fewer TS cache entries because it does not chop\n  up intervals that were superseded in the middle. These chops require\n  inserting a whole new cache entry which requires allocations. However,\n  only **.05%** of insertions are split in this PR, so it doesn't seem like this\n  is causing an issue.\n- **32%** of insertions resulting in a direct replacement here vs **~0%** there.\n- **.25%** of insertions resulting in a deletion here vs **25%** there.\n\nPerf Impact:\n\n```\nname                      old time/op    new time/op    delta\nBank_Cockroach-4             313µs ± 2%     339µs ±10%   +8.29%         (p=0.016 n=8+10)\nSelect1_Cockroach-4         52.7µs ± 1%    52.6µs ± 1%     ~            (p=0.549 n=9+10)\nSelect2_Cockroach-4          791µs ± 1%     775µs ± 1%   -2.07%          (p=0.000 n=9+9)\nInsert1_Cockroach-4          393µs ±17%     371µs ± 2%     ~           (p=0.190 n=10+10)\nInsert10_Cockroach-4         660µs ± 2%     650µs ± 0%   -1.48%          (p=0.001 n=8+9)\nInsert100_Cockroach-4       3.39ms ± 1%    3.33ms ± 0%   -1.51%         (p=0.000 n=10+9)\nInsert1000_Cockroach-4      33.1ms ± 7%    31.8ms ± 1%   -4.03%         (p=0.000 n=9+10)\nUpdate1_Cockroach-4          591µs ± 9%     566µs ± 1%     ~             (p=0.063 n=9+9)\nUpdate10_Cockroach-4        1.11ms ± 1%    1.10ms ± 0%     ~             (p=0.321 n=9+8)\nUpdate100_Cockroach-4       6.12ms ± 5%    5.98ms ± 1%   -2.30%          (p=0.002 n=8+8)\nUpdate1000_Cockroach-4      55.1ms ±10%    50.9ms ± 1%   -7.73%         (p=0.000 n=9+10)\nDelete1_Cockroach-4          785µs ±22%     695µs ± 2%  -11.46%        (p=0.035 n=10+10)\nDelete10_Cockroach-4        1.98ms ± 4%    2.09ms ± 3%   +5.36%        (p=0.000 n=10+10)\nDelete100_Cockroach-4       18.0ms ±11%    17.3ms ± 1%     ~           (p=0.143 n=10+10)\nDelete1000_Cockroach-4       144ms ± 5%     162ms ± 1%  +12.91%         (p=0.000 n=9+10)\nScan1_Cockroach-4            199µs ± 9%     183µs ± 1%   -7.83%        (p=0.015 n=10+10)\nScan10_Cockroach-4           241µs ±15%     215µs ± 1%  -10.74%         (p=0.000 n=10+9)\nScan100_Cockroach-4          478µs ± 1%     473µs ± 1%   -1.21%         (p=0.000 n=8+10)\nScan1000_Cockroach-4        3.12ms ± 5%    2.97ms ± 1%   -4.93%        (p=0.000 n=10+10)\nScan10000_Cockroach-4       32.2ms ± 3%    30.4ms ± 1%   -5.42%         (p=0.000 n=10+9)\nPgbenchExec_Cockroach-4     2.83ms ±16%    2.75ms ± 5%     ~           (p=0.684 n=10+10)\nPgbenchQuery_Cockroach-4    2.68ms ± 5%    2.63ms ± 3%     ~            (p=0.173 n=8+10)\n\nname                      old alloc/op   new alloc/op   delta\nBank_Cockroach-4            69.1kB ± 1%    69.2kB ± 1%     ~           (p=0.403 n=10+10)\nSelect1_Cockroach-4         3.90kB ± 0%    3.90kB ± 0%     ~            (p=1.172 n=9+10)\nSelect2_Cockroach-4         80.5kB ± 0%    80.5kB ± 0%   -0.04%        (p=0.022 n=10+10)\nInsert1_Cockroach-4         22.8kB ± 0%    22.7kB ± 0%   -0.12%        (p=0.023 n=10+10)\nInsert10_Cockroach-4        66.8kB ± 0%    66.8kB ± 0%     ~           (p=0.565 n=10+10)\nInsert100_Cockroach-4        506kB ± 0%     505kB ± 0%     ~            (p=0.156 n=10+9)\nInsert1000_Cockroach-4      4.53MB ± 1%    4.51MB ± 0%   -0.51%         (p=0.008 n=10+9)\nUpdate1_Cockroach-4         36.4kB ± 0%    36.4kB ± 0%     ~           (p=0.739 n=10+10)\nUpdate10_Cockroach-4        98.8kB ± 0%    98.8kB ± 0%     ~           (p=0.382 n=10+10)\nUpdate100_Cockroach-4        701kB ± 0%     699kB ± 0%   -0.30%        (p=0.023 n=10+10)\nUpdate1000_Cockroach-4      6.05MB ± 0%    6.06MB ± 1%     ~            (p=0.780 n=9+10)\nDelete1_Cockroach-4         35.3kB ± 0%    35.6kB ± 0%   +0.79%         (p=0.000 n=10+9)\nDelete10_Cockroach-4         105kB ± 0%     108kB ± 0%   +2.56%         (p=0.000 n=10+9)\nDelete100_Cockroach-4        788kB ± 0%     815kB ± 0%   +3.33%          (p=0.000 n=9+9)\nDelete1000_Cockroach-4      7.13MB ± 1%    7.37MB ± 1%   +3.33%        (p=0.000 n=10+10)\nScan1_Cockroach-4           13.1kB ± 0%    13.1kB ± 0%     ~           (p=0.810 n=10+10)\nScan10_Cockroach-4          17.0kB ± 0%    17.0kB ± 0%   -0.15%         (p=0.041 n=9+10)\nScan100_Cockroach-4         49.2kB ± 0%    49.2kB ± 0%     ~           (p=0.955 n=10+10)\nScan1000_Cockroach-4         331kB ± 0%     331kB ± 0%   -0.04%        (p=0.000 n=10+10)\nScan10000_Cockroach-4       5.74MB ± 0%    5.72MB ± 0%   -0.37%        (p=0.000 n=10+10)\nPgbenchExec_Cockroach-4      169kB ± 9%     174kB ±12%     ~             (p=0.387 n=9+9)\nPgbenchQuery_Cockroach-4     213kB ± 0%     213kB ± 0%   +0.07%         (p=0.001 n=8+10)\n\nname                      old allocs/op  new allocs/op  delta\nBank_Cockroach-4             1.67k ± 0%     1.68k ± 1%     ~            (p=0.633 n=8+10)\nSelect1_Cockroach-4           97.0 ± 0%      97.0 ± 0%     ~     (all samples are equal)\nSelect2_Cockroach-4          2.70k ± 0%     2.70k ± 0%     ~     (all samples are equal)\nInsert1_Cockroach-4            352 ± 0%       352 ± 0%   -0.14%         (p=0.022 n=9+10)\nInsert10_Cockroach-4           903 ± 0%       902 ± 0%     ~           (p=0.720 n=10+10)\nInsert100_Cockroach-4        6.17k ± 0%     6.17k ± 0%     ~            (p=0.249 n=10+9)\nInsert1000_Cockroach-4       58.8k ± 0%     58.7k ± 0%   -0.20%         (p=0.020 n=10+9)\nUpdate1_Cockroach-4            712 ± 0%       712 ± 0%     ~     (all samples are equal)\nUpdate10_Cockroach-4         1.40k ± 0%     1.40k ± 0%     ~           (p=0.720 n=10+10)\nUpdate100_Cockroach-4        7.93k ± 0%     7.90k ± 0%     ~           (p=0.057 n=10+10)\nUpdate1000_Cockroach-4       70.0k ± 0%     70.0k ± 1%     ~            (p=1.000 n=9+10)\nDelete1_Cockroach-4            681 ± 0%       684 ± 0%   +0.46%        (p=0.000 n=10+10)\nDelete10_Cockroach-4         1.66k ± 0%     1.69k ± 0%   +1.79%        (p=0.000 n=10+10)\nDelete100_Cockroach-4        11.1k ± 0%     11.4k ± 0%   +2.66%          (p=0.000 n=8+8)\nDelete1000_Cockroach-4        106k ± 0%      108k ± 0%   +2.52%        (p=0.000 n=10+10)\nScan1_Cockroach-4              272 ± 0%       272 ± 0%     ~     (all samples are equal)\nScan10_Cockroach-4             352 ± 0%       352 ± 0%     ~           (p=0.577 n=10+10)\nScan100_Cockroach-4          1.08k ± 0%     1.08k ± 0%     ~           (p=1.000 n=10+10)\nScan1000_Cockroach-4         8.29k ± 0%     8.29k ± 0%     ~     (all samples are equal)\nScan10000_Cockroach-4        80.4k ± 0%     80.4k ± 0%   +0.01%         (p=0.000 n=10+9)\nPgbenchExec_Cockroach-4      3.29k ±21%     3.37k ±17%     ~           (p=0.436 n=10+10)\nPgbenchQuery_Cockroach-4     3.87k ± 0%     3.88k ± 0%   +0.23%         (p=0.000 n=8+10)\n```","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/c80009ce02f018b504a7d84f73489ed3d60022ff"},{"sha":"77be084ff0de3e7704e2e4f8b2b2911eb3c067e3","author":{"email":"885ea01d97daf7784d4a0ae7f66c69a3686faeb6@gmail.com","name":"Nathan VanBenschoten"},"message":"storage: IntervalCache keeps ordering correct when adjusting keys\n\nWhen we adjust intervals in the `TimestampCache`, we need to make sure\nthe ordering in the FIFO queue stays consistent with interval timestamps.\nFor instace, if we don't move the interval back in the queue in the value\nreplacement case, intervals will get to a point where their eviction\nis blocked on the newly replaced interval. This can lead to memory\ngrowth of upwards of **40 MB**!\n\nThis change includes the following `util/cache` methods to help with\nthis:\n- (*baseCache).AddEntryAfter\n- (*baseCache).MoveToEnd","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/77be084ff0de3e7704e2e4f8b2b2911eb3c067e3"},{"sha":"1a6ccc3886d539c79082e8fe0402d65f3e0442be","author":{"email":"885ea01d97daf7784d4a0ae7f66c69a3686faeb6@gmail.com","name":"Nathan VanBenschoten"},"message":"util/cache: Allow cache DelEntry to avoid lookup\n\nThis change makes a few changes to `util/cache.baseCache` to allow\ndirect usageof `*cache.Entry` values when possible when deleting entries.\nThis allows us to avoid Entry lookup.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/1a6ccc3886d539c79082e8fe0402d65f3e0442be"},{"sha":"7616617565748120676b1704509b5fafbf39acb6","author":{"email":"b25b72a65195e46aa49449589ffbea257ff840a3@gmail.com","name":"Peter Mattis"},"message":"cli: move the range/kv commands under debug\n\nRemove the \"range merge\" command which is not fully supported on the\nserver side.\n\nFixes #4751.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/7616617565748120676b1704509b5fafbf39acb6"},{"sha":"e3af593418c8ba81a4bcdc1f3f770b3e57637858","author":{"email":"8dc7c09356db0b1762fb140617d7375c0a5a4131@cockroachlabs.com","name":"Andrei Matei"},"message":"sql: match postgres behavior in handling of errors on COMMIT\n\nThe txn needs to not remain in the aborted state if a COMMIT is executed\nand errors, as there's no way for the client to recover (e.g. in go you\ncan't use a Txn object after doing Txn.Commit()). As before this change,\nan error is still returned to the client.\n\nKeep in mind that statements in the same batch coming after a failed\nCOMMIT are still not executed; that's in line with the principle of not\nrunning any statements after an error since we can't return results for\nthem.\n\nThe change pushes the responsibility of updating txnState down from\nexecStmtsInCurrentTxn() to execStmtInCurrentTxn(). The actual change in\nbehavior comes from setting txnState.aborted = false in the `txnDone`\ncase. Before, the error would take precedence and txnState.aborted would\nbe set to true.\n\nFixes #4752","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/e3af593418c8ba81a4bcdc1f3f770b3e57637858"},{"sha":"d9cf809047bdb170e7f5c7ea260384d11aba5cb9","author":{"email":"c96a05aaafbe4b3abc1841a4083a1b0f402c2ba6@gmail.com","name":"Tamir Duberstein"},"message":"server: remove this task that likes to deadlock","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/d9cf809047bdb170e7f5c7ea260384d11aba5cb9"},{"sha":"9f97627f92762b1a6573f9e1ef3500dfad43d1da","author":{"email":"b25b72a65195e46aa49449589ffbea257ff840a3@gmail.com","name":"Peter Mattis"},"message":"cli: improve usage text\n\nUse different usage text for the --port and --host flags for different\ncommands.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/9f97627f92762b1a6573f9e1ef3500dfad43d1da"},{"sha":"7f7270a66a88a02aae527cc3b6a0ed0406030e72","author":{"email":"b25b72a65195e46aa49449589ffbea257ff840a3@gmail.com","name":"Peter Mattis"},"message":"cli: remove --log-dir default value help text\n\nFixes #4816.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/7f7270a66a88a02aae527cc3b6a0ed0406030e72"},{"sha":"3594cbc94a750f6d9510d4a7be2f84660f595b51","author":{"email":"3a2f0da2a0c450062f2df795b8346ad513a9734a@gmail.com","name":"Daniel Harrison"},"message":"Implement SQL UNION.\n\nDoes not yet work with ORDER BY or LIMIT on the UNION.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/3594cbc94a750f6d9510d4a7be2f84660f595b51"},{"sha":"efc4dd38673d29426d0b13688ff3eb43d20effbc","author":{"email":"0706025b2bbcec1ed8d64822f4eccd96314938d0@cockroachlabs.com","name":"Max Lang"},"message":"fix broken overview for secure mode","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/efc4dd38673d29426d0b13688ff3eb43d20effbc"},{"sha":"ecac1bb0c463e4fb6d2f90e0412178916540b04f","author":{"email":"4b8373d016f277527198385ba72fda0feb5da015@cockroachlabs.com","name":"Peter Mattis"},"message":"Merge pull request #4811 from petermattis/pmattis/usage-text\n\ncli: improve usage text","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/ecac1bb0c463e4fb6d2f90e0412178916540b04f"},{"sha":"2f5bb76eab71d2fd5431949aed23309ba86a253e","author":{"email":"4b8373d016f277527198385ba72fda0feb5da015@cockroachlabs.com","name":"Peter Mattis"},"message":"Merge pull request #4812 from petermattis/pmattis/range-kv-cmds\n\ncli: move the range/kv commands under debug","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/2f5bb76eab71d2fd5431949aed23309ba86a253e"},{"sha":"4f4f6d0929d6f247c385813562c4b90121c95af7","author":{"email":"3a2f0da2a0c450062f2df795b8346ad513a9734a@gmail.com","name":"Dan Harrison"},"message":"Merge pull request #4764 from paperstreet/union\n\nImplement SQL UNION","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/4f4f6d0929d6f247c385813562c4b90121c95af7"},{"sha":"2629ff75b8edf2389181f2c270d000c3c6cc88ad","author":{"email":"885ea01d97daf7784d4a0ae7f66c69a3686faeb6@gmail.com","name":"Nathan VanBenschoten"},"message":"util/cache: Fix baseCahce.Clear by walking linked list and dumping store\n\nBefore this change, there was a subtle bug where `IntervalCache.clear`\nwould fail to clear all entries from the cache. This issue came about\nwhen interval keys had been modified in the cache in certain ways, such\nthat when deleting entries from the underlying red-black tree (in an\nin-order depth-first ordering provided by `tree.Do`), rotations occurred.\nIf a rotation occurred, the traversal could be messed up and nodes could\nbe missed.\n\nThis change fixes this issue by simply replacing the entire tree on\nclear, which should also be more efficient. It also includes a repro test\ncase for the issue it fixes.\n\nIt follows this pattern with the `OrderedCache` and `UnorderedCache` by\nadding a new `init` method to the `cacheStore` interface and removing\n`del` from the interface. A mildly noteworthy part of this change is\nthat it requires us to now use the `baseCache`'s linked list even if\nthe eviction policy is set to `CacheNone`.","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/2629ff75b8edf2389181f2c270d000c3c6cc88ad"},{"sha":"ffa5a23fd88e4271944eade3cb18a38c01c22ac3","author":{"email":"5a8372f5628deb22ab258f403701c7c55f2b695f@gmail.com","name":"Andrei Matei"},"message":"Merge pull request #4779 from andreimatei/debug-concurrent-increment\n\nsql: match postgres behavior in handling of errors on COMMIT","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/ffa5a23fd88e4271944eade3cb18a38c01c22ac3"},{"sha":"07d30ac927f59b5d095c9a55f336c599c85756e9","author":{"email":"0377ed3530dda2b0c8356d5ae4c608082561695d@gmail.com","name":"Max Lang"},"message":"Merge pull request #4809 from maxlang/maxlang/ui-fix-broken-secure\n\nfix broken overview for secure mode","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/07d30ac927f59b5d095c9a55f336c599c85756e9"},{"sha":"58f8e0de9de1274e876bc06213958aaa712f0e75","author":{"email":"0706025b2bbcec1ed8d64822f4eccd96314938d0@cockroachlabs.com","name":"Max Lang"},"message":"wire up endpoints for dbs and and add table pages","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/58f8e0de9de1274e876bc06213958aaa712f0e75"},{"sha":"c239162dcb5e5da0d7167ac976f830dbda368709","author":{"email":"885ea01d97daf7784d4a0ae7f66c69a3686faeb6@gmail.com","name":"Nathan VanBenschoten"},"message":"Merge pull request #4789 from nvanbenschoten/nvanbenschoten/tsCacheChop\n\nstorage: Trim overlapping timestamp intervals in TimestampCache","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/c239162dcb5e5da0d7167ac976f830dbda368709"},{"sha":"c832e618aa731ae8f9914cdc1ee660efb0c15348","author":{"email":"c96a05aaafbe4b3abc1841a4083a1b0f402c2ba6@gmail.com","name":"Tamir Duberstein"},"message":"Merge pull request #4813 from tamird/remove-deadlocker-task\n\nserver: remove this task that likes to deadlock","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/c832e618aa731ae8f9914cdc1ee660efb0c15348"},{"sha":"beaf01d4ca66755f41407cf81dcf4bf8feb66ea3","author":{"email":"4b8373d016f277527198385ba72fda0feb5da015@cockroachlabs.com","name":"Peter Mattis"},"message":"Merge pull request #4817 from petermattis/pmattis/log-dir-default-help-text\n\ncli: remove --log-dir default value help text","distinct":false,"url":"https://api.github.com/repos/cockroachdb/cockroach/commits/beaf01d4ca66755f41407cf81dcf4bf8feb66ea3"}]}