Bug 206912 - databases/mysql57-servers: performance_schema disabled by default
Summary: databases/mysql57-servers: performance_schema disabled by default
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Thomas Zander
URL:
Keywords: needs-qa, patch-ready
Depends on: 205956
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-04 08:59 UTC by Steven Hartland
Modified: 2016-02-08 19:15 UTC (History)
2 users (show)

See Also:
riggs: merge-quarterly+


Attachments
patch for mysql57-server adds perf_schema disable/enable option install time (2.53 KB, patch)
2016-02-04 16:33 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
Revised patch using OPTION GROUPs (2.32 KB, patch)
2016-02-06 20:14 UTC, Thomas Zander
mmokhi: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Hartland freebsd_committer 2016-02-04 08:59:43 UTC
The performance_schema really shouldn't be disabled by default, its very useful feature, which is key to diagnosing issues on the fly.

If a user doesn't have enough memory then, by all means provide the option to disable it but this shouldn't be the default.
Comment 1 Steven Hartland freebsd_committer 2016-02-04 09:00:18 UTC
For reference this was changed yesterday by: r408012
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-04 09:42:35 UTC
(In reply to Steven Hartland from comment #1)

Can this now be closed?

If so, please assign to yourself
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-04 09:43:48 UTC
Oops, I meant Thomas (riggs) for the last comment
Comment 4 Steven Hartland freebsd_committer 2016-02-04 12:44:48 UTC
(In reply to Kubilay Kocak from comment #2)
Sorry to be clear this was broken by yesterday commit r408012
Comment 5 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-04 13:15:27 UTC
(In reply to Steven Hartland from comment #4)
Steven: Many people requested me to do this (disable by default in configs), as it eats about 800MB RAM by fresh install !

BTW, as both feedbacks seems reasonable to me (:D), im gonna add install time option to choose.
And it's obvious that user can change it by configs at any time (this is only default config not compile/built-in setting)
Comment 6 Steven Hartland freebsd_committer 2016-02-04 13:56:43 UTC
(In reply to Mahdi Mokhtari from comment #5)
Sorry but 800MB of RAM is nothing in a production machine.

If you're not just playing with or its a dev instance, then by all means provide a port option to disable it by default.

For those that don't want it at runtime they can add the following to my.cnf:
performance_schema=OFF

There's a good reason why the default has changed ;-)
Comment 7 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-04 15:11:50 UTC
(In reply to Steven Hartland from comment #6)
I knew it and ive suggested those ones to do that :)

BTW, as i told, im gonna enabling it again (mostly because of MySQL-dev guys suggestions and your argument [production]), AND im putting install-time option to make it disable by default :D
in this way i can respect both ideas (low-resources/productions)
Comment 8 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-04 16:33:11 UTC
Created attachment 166572 [details]
patch for mysql57-server adds perf_schema disable/enable option install time

patch adds perf_schema disable/enable option install time.
it also enables that by default :D
Comment 9 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-04 16:33:49 UTC
@Thomas (riggs)

Up to you now, mate :D
Comment 10 Thomas Zander freebsd_committer 2016-02-06 19:12:11 UTC
Currently double-checking pkg builds...
Comment 11 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-06 19:51:38 UTC
(In reply to Thomas Zander from comment #10)
Thanks :)
Comment 12 Thomas Zander freebsd_committer 2016-02-06 20:14:53 UTC
Created attachment 166667 [details]
Revised patch using OPTION GROUPs

If I read your patch correctly, you want to have a new GROUP for essentially unrelated features instead of a RADIO.
Attached proposal changes your patch accordingly. Can you review if this is what you intended?

Generally, please consider if it is really necessary to introduce OPTION knobs for features that can be enabled or disabled via a config file and do not introduce LIB_/RUN_DEPENDS.
With every new OPTION that you introduce, more permutations of OPTIONs are possible -> More testing for the maintainer / committer prior to updates of the port Makefile, more potential ways to break the port in an unexpected way.

Honestly, my recommendation would be to support one default behaviour for the perf schema, remove the knob entirely and have users change the behaviour via the configuration file if they choose to do so.
Comment 13 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-06 21:05:28 UTC
Comment on attachment 166667 [details]
Revised patch using OPTION GROUPs

(In reply to Thomas Zander from comment #12)
I've read your edit on patch, changed RADIO to GROUP, yes?
Oh using FEATURS instead of FEATURES was intentional (to keep lines on one line, no matter :D).

im okay with it :), (and ill approve it :D) 

> Honestly, my recommendation would be to support one default behaviour
I was to do this.
But honestly i've received from feedback of users (on FreeBSD community and even on MySQL community) this is a debate between production (high resource) and low resource users.
And i was aware of how good (and also RAM EATING) feature perf_schema is :)
So i think i had no better choice to satisfy (and respect) both big groups of users.
**also if user don't choice, enable is used by default.
Comment 14 commit-hook freebsd_committer 2016-02-07 08:08:58 UTC
A commit references this bug:

Author: riggs
Date: Sun Feb  7 08:08:21 UTC 2016
New revision: 408349
URL: https://svnweb.freebsd.org/changeset/ports/408349

Log:
  Revert recent change on performance_schema; introduce OPTION for it

  The recent port version 5.7.10_2 introduced a change in the default
  behaviour of performance_schema. Due to an ongoing debate in the
  community whether the default setting should lean towards performance
  (previous default before 5.7.10_2) or memory consumption, maintainer
  had changed the default to memory consumption in 5.7.10_2.
  This introduces an OPTION knob PERFSCHM to control the default behaviour
  of performance_schema. It defaults to ON, hence restoring the previous
  default.
  Bump PORTREVISION.

  PR:		206912
  Submitted by:	smh
  Reviewed by:	mokhi64@gmail.com (maintainer), riggs
  Approved by:	mokhi64@gmail.com (maintainer)
  MFH:		2016Q1

Changes:
  head/databases/mysql57-server/Makefile
  head/databases/mysql57-server/files/mysql-server.in
  head/databases/mysql57-server/pkg-message
Comment 15 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-07 08:25:10 UTC
Thanks :)
Comment 16 commit-hook freebsd_committer 2016-02-08 18:42:14 UTC
A commit references this bug:

Author: riggs
Date: Mon Feb  8 18:41:55 UTC 2016
New revision: 408495
URL: https://svnweb.freebsd.org/changeset/ports/408495

Log:
  MFH: r408349

  Revert recent change on performance_schema; introduce OPTION for it

  The recent port version 5.7.10_2 introduced a change in the default
  behaviour of performance_schema. Due to an ongoing debate in the
  community whether the default setting should lean towards performance
  (previous default before 5.7.10_2) or memory consumption, maintainer
  had changed the default to memory consumption in 5.7.10_2.
  This introduces an OPTION knob PERFSCHM to control the default behaviour
  of performance_schema. It defaults to ON, hence restoring the previous
  default.
  Bump PORTREVISION.

  PR:		206912
  Submitted by:	smh
  Reviewed by:	mokhi64@gmail.com (maintainer), riggs
  Approved by:	ports-secteam (feld), mokhi64@gmail.com (maintainer)

Changes:
_U  branches/2016Q1/
  branches/2016Q1/databases/mysql57-server/Makefile
  branches/2016Q1/databases/mysql57-server/files/mysql-server.in
  branches/2016Q1/databases/mysql57-server/pkg-message
Comment 17 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-08 19:15:21 UTC
(In reply to commit-hook from comment #16)
Thanks :D