8000 Deprecate rocksdb.max-write-buffer-number startup option by Simran-B · Pull Request #9654 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Deprecate rocksdb.max-write-buffer-number startup option #9654

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
Merged
merged 5 commits into from
Oct 9, 2019
Merged

Deprecate rocksdb.max-write-buffer-number startup option #9654

merged 5 commits into from
Oct 9, 2019

Conversation

Simran-B
Copy link
Contributor
@Simran-B Simran-B commented Aug 7, 2019

Scope & Purpose

Mark the startup option rocksdb.max-write-buffer-number as deprecated in v3.4.8 and v3.5.0 because it does nothing. It is hardcoded to number of column families (7) + 2 = 9.

matthew:

Set too low it causes rocksdb to regularly block all writes. Yes, it could be set higher ... but can also lead to block writes. Currently the value is fixed at "number of column families" plus two. This setting was heavily tested. And shown to be aligned with five years of research on Google's leveldb upon which rocksdb is based.

  • Backport to 3.4 and 3.5

Related Information

(Please reference tickets / specification etc )

Testing & Verification

This change is a trivial rework / code cleanup without any test coverage - while technically a code change, it should affect nothing but the documentation (and arangod --help output)

@Simran-B Simran-B added this to the devel milestone Aug 7, 2019
@Simran-B Simran-B self-assigned this Aug 7, 2019
@Simran-B
Copy link
Contributor Author
Simran-B commented Aug 7, 2019

Code change verified to be working, arangod.json (for backport):

  "rocksdb.max-write-buffer-number" : {
    "category" : "option",
    "default" : 2,
    "deprecatedIn" : [
      "v3.4.8",
      "v3.5.0"
    ],
    "description" : "maximum number of write buffers that build up in memory. This option is ignored! There is a hardcoded maximum of number of column families + 2 (currently 9) write buffers based on performance research",
    "dynamic" : false,
    "enterpriseOnly" : false,
    "hidden" : false,
    "introducedIn" : null,
    "obsolete" : false,
    "requiresValue" : true,
    "section" : "rocksdb",
    "type" : "uint64"
  },

@Simran-B Simran-B changed the title Deprecate for v3.4.8 and v3.5.0 Deprecate rocksdb.max-write-buffer-number startup option Aug 19, 2019
@Simran-B
Copy link
Contributor Author

@jsteemann says that we should think about making this option actually work instead of deprecating it.
8000

@matthewvon
Copy link
Contributor

"make it work" then means the minimum should be number of column families plus two, and user could increase above that. Going lower creates performance bottleneck on write buffer flush.

@Simran-B
Copy link
Contributor Author

Okay, but I think the value the user passes should be the total max, and ignored if it's below column families plus two (with a log warning). The current default seems to be the offset of plus two? That's kinda confusing, especially because the true max is 9 by default.

Will leave this PR pending until further notice.

@matthewvon
Copy link
Contributor

"true max" is 2: https://github.com/arangodb/arangodb/blob/devel/3rdParty/rocksdb/6.2/include/rocksdb/advanced_options.h#L167 ... I am the person that raised it to 9 ... 7 column families plus 2.

@jsteemann
Copy link
Contributor

Let's just honor the user's setting for this option if it is explicitly set by the user.
And if it's not explicitly set, then we should apply our default (which currently is 7+2).
Objections?

@matthewvon
Copy link
Contributor

Yes. I object to allowing less than 9. The user is not going to understand the performance impact. They are likely thinking "this will decrease my memory footprint". Then we end up having to debug their environment when server suddenly has periodic stalls which can lead to cluster replication sync issues and other nasty items.

jsteemann added a commit that referenced this pull request Aug 19, 2019
@jsteemann
issue #9654: make --rocksdb.max-write-buffer-number work
f28d179
jsteemann added a commit that referenced this pull request Aug 19, 2019
@jsteemann
issue #9654: make --rocksdb.max-write-buffer-number work
edbcfec
This was referenced Aug 19, 2019
Merged
Merged
jsteemann added a commit that referenced this pull request Aug 19, 2019
@jsteemann
issue #9654
279c760
matthewvon pushed a commit that referenced this pull request Aug 19, 2019
@jsteemann @matthewvon
issue #9654: make --rocksdb.max-write-buffer-number work (#9750)
346edae 
* issue #9654: make `--rocksdb.max-write-buffer-number` work

* fix the logic
@jsteemann jsteemann mentioned this pull request Aug 19, 2019
Merged
4 tasks
@jsteemann
Copy link
Contributor

I think the above PRs of mine obsolete this issue. @Simran-B

KVS85 pushed a commit that referenced this pull request Aug 19, 2019
@jsteemann @KVS85
issue #9654 (#9752)
eb5f424
KVS85 pushed a commit that referenced this pull request Aug 20, 2019
@jsteemann @KVS85
issue #9654: make --rocksdb.max-write-buffer-number work (#9751)
0f03655 
* issue #9654: make `--rocksdb.max-write-buffer-number` work

* fix the logic
ObiWahn added a commit that referenced this pull request Aug 22, 2019
@ObiWahn
Merge branch 'devel' of https://github.com/arangodb/arangodb into fea…
12d698d 
…ture/mimalloc

* 'devel' of https://github.com/arangodb/arangodb: (83 commits)
  Bug fix/internal issue #622 (#9781)
  mark AQL functions FULLTEXT, NEAR, WITHIN, WITHIN_RECTANGLE as cacheable (#9771)
  reduce wait timeout, use move
  Bug fix/implement windows maintenance tests (#9763)
  show query string length and cacheability in explain output (#9767)
  fix potential spurious wakeups in scheduler code (#9770)
  Bug fix/issue #9612 (#9764)
  downgrade WARN messages to INFO level (#9761)
  slightly reorder boolean members to reduce struct sizes (#9762)
  make index selection more deterministic (#9735)
  Update PULL_REQUEST_TEMPLATE.md (#9758)
  [devel] Move Shard Bug 4567124 (#9746)
  Bug fix/fix signed int overflow (#9717)
  Bug fix/fix invalid cast (#9755)
  Enforce stricter transaction limits (#9740)
  Check scheduler queue return value (#9754)
  dont fill cache on truncate (#9721)
  fix pasting from the documentation (#9742)
  tell that procdump is gone - it seems this happenes in reality without coredumps being written (#9748)
  issue #9654: make `--rocksdb.max-write-buffer-number` work (#9750)
  ...
@Simran-B Simran-B requested review from jsteemann and matthewvon and removed request for fceller and joerg84 August 30, 2019 13:05
@Simran-B
Copy link
Contributor Author

Removed the deprecation part, reworded the description to:

maximum number of write buffers that build up in memory (default: number of column families + 2 = 9 write buffers). You can increase the amount only

@Simran-B Simran-B force-pushed the deprecate-rocksdb-max-write-buffer-number branch from 194b6fd to e45cf42 Compare October 9, 2019 21:32
@Simran-B Simran-B merged commit a8bb6dc into devel Oct 9, 2019
@Simran-B Simran-B deleted the deprecate-rocksdb-max-write-buffer-number branch October 9, 2019 21:33
ObiWahn added a commit that referenced this pull request Oct 11, 2019
@ObiWahn
Merge branch 'devel' of https://github.com/arangodb/arangodb into fea…
7f67ae6 
…ture/one-shard-clean-up-2

* 'devel' of https://github.com/arangodb/arangodb:
  Bug fix/improve stringutils performance (#10208)
  add option to talk to the SUT using VST (#10217)
  Doc - Added "log-output" example (#10207)
  fix it! (#10198)
  add missing include
  Bug fix/fix simple example dep proxy skip some regression test (#10213)
  fixed ui behaviour when replacing a foxx app (#9719)
  [devel] Fix document search (Ctrl+F/Cmd+F) (#10216)
  Convert many uses of ClusterComm to Fuerte (#10154)
  Remove invokeOnAllElements (#10212)
  AQL Subquery: MultiDependencyRowFetcher (#10101)
  Bug fix/fix remote executor races (#10206)
  fix several inefficiencies in Store (#10189)
  Deprecate rocksdb.max-write-buffer-number startup option (#9654)
  fix arangosh with vst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewvon matthewvon matthewvon approved these changes

@jsteemann jsteemann Awaiting requested review from jsteemann

Assignees

@jsteemann jsteemann

@Simran-B Simran-B

Labels
3 Documentation
Projects
None yet
Milestone
devel
Development

Successfully merging this pull request may close these issues.
3 participants
@Simran-B @matthewvon @jsteemann
0