| Bug #47897 | Unnecessary, non-public debug definitions in MGMAPI | ||
|---|---|---|---|
| Submitted: | 7 Oct 2009 15:30 | Modified: | 9 Oct 2009 9:56 |
| Reporter: | Martin Zaun | Email Updates: | |
| Status: | Patch approved | Impact on me: | |
| Category: | MySQL Cluster: NDB API | Severity: | S4 (Feature request) |
| Version: | mysql-5.1-telco-6.2 | OS: | Any |
| Assigned to: | Assigned Account | CPU Architecture: | Any |
| Tags: | cleanup, ndb | ||
[8 Oct 2009 16:47]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/86225 3093 Martin Zaun 2009-10-08 ndb - bug#47897 - minor clean up actions in include/mgmapi.
[8 Oct 2009 17:00]
Martin Zaun
Hi Magnus,
the patch differs slightly from our initial email discussion:
1) straight forward
not needed: mgmapi_config_parameters_debug.h
moved: mgmapi_debug.h
2) fixed
3) turns out rather impractical:
- the NODE_TYPE... defs are used right there in mgmapi.h
- the CFG_ defs semantically belong into mgmapi_config_parameters.h
- moving CONNECTION_TYPE defs to mgmapi_internal.h (or _debug.h)
would require creating new include dependencies upon src/mgmapi
in these files
src/common/mgmcommon/ConfigRetriever.cpp
src/common/mgmcommon/IPCConfig.cpp
src/mgmapi/mgmapi_internal.h
src/mgmsrv/ConfigInfo.cpp
tools/ndb_config.cpp
This just does not "feel" right to me.
So, I reverted to the original idea leaving them in place but
flagging them with DOXYGEN_SHOULD_SKIP_INTERNAL. Minimal intrusion.
4) I'd prefer the DOXYGEN_SHOULD_SKIP_DEPRECATED flag on the deprecated
field even if it means a bit more clutter. I'll undo, however, if
you object.
Thanks,
Martin
[8 Oct 2009 17:02]
Martin Zaun
P.S.: Compiled and tested in 6.3 with mtr --suite=ndb,rpl_ndb,ndb_binlog.
[9 Oct 2009 9:56]
Magnus Blåudd
Suggest you do a "make distcheck" as well. It will detect if you missed any Makefile updates. In 7.0, the new include paths will need to be added to CMakeLists.txt as well.
[9 Oct 2009 17:11]
Martin Zaun
Hi Magnus,
on my system, 'make distcheck' aborts with many errors, already on a *clean* ndb-6.3 branch:
gcc -DHAVE_CONFIG_H -I. -I../../include -I../../../cmd-line-utils/libedit -I../../include -I../../../include -O -D_P1003_1B_VISIBLE -DSIGNAL_WITH_VIO_CLOSE -DSIGNALS_DONT_BREAK_READ -DIGNORE_SIGHUP_SIGQUIT -DDONT_DECLARE_CXA_PURE_VIRTUAL -MT help.o -MD -MP -MF .deps/help.Tpo -c -o help.o help.c
In file included from ../../../cmd-line-utils/libedit/el.h:96,
from help.c:3:
../../../cmd-line-utils/libedit/tty.h:459: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:460: error: syntax error before ‘void’
../../../cmd-line-utils/libedit/tty.h:461: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:462: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:463: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:464: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:465: error: syntax error before ‘int’
../../../cmd-line-utils/libedit/tty.h:466: error: syntax error before ‘void’
In file included from ../../../cmd-line-utils/libedit/el.h:97,
...
In my patched work branch, 'make distclean' aborts with the same error; so, it's not related to my patch.
Please, advise.
Will add include changes to CMakeLists.txt in >= 7.0.
[16 Oct 2009 8:21]
Magnus Blåudd
Seems like the compiler error you get is in our bundled version of libedit. We have the option to use bundled libedit, bundled readline or system installed one. Looks like "make distcheck" always want to compile bundled libedit. Ignore it and go ahead with your push.
[28 Oct 2009 15:30]
Magnus Blåudd
BUG#24809 contains a fix to the "make distcheck" problem
Description: This is not a bug but an NDB MGM API code cleanup item. Wrapping the MGMAPI with a JTie Java interface has revealed some unnecessary definitions in the .h files that are not meant to be part of the public API. 1) These two files are for internal debugging purposes: mgmapi_config_parameters_debug.h mgmapi_debug.h Move these files from include/mgmapi/ to /src/mgmapi/. 2) Delete duplicate definitions in mgmapi_config_parameters.h: #define CFG_DB_DISK_PAGE_BUFFER_MEMORY 160 /* used from 5.1 */ #define CFG_DB_STRING_MEMORY 161 /* used from 5.1 */ #define CFG_DB_DISK_PAGE_BUFFER_MEMORY 160 #define CFG_DB_STRING_MEMORY 161 3) Move internal definitions in mgmapi_config_parameters.h to mgmapi_config_parameters_debug.h: /** * Internal */ #define CFG_DB_STOP_ON_ERROR_INSERT 1 #define CFG_TYPE_OF_SECTION 999 #define CFG_SECTION_SYSTEM 1000 #define CFG_SECTION_NODE 2000 #define CFG_SECTION_CONNECTION 3000 #define NODE_TYPE_DB 0 #define NODE_TYPE_API 1 #define NODE_TYPE_MGM 2 #define CONNECTION_TYPE_TCP 0 #define CONNECTION_TYPE_SHM 1 #define CONNECTION_TYPE_SCI 2 #define CONNECTION_TYPE_OSE 3 /* Removed. */ 4) Consider demarcating deprecated field in log_event.h with flag DOXYGEN_SHOULD_SKIP_DEPRECATED: struct ndb_logevent_MemoryUsage { int gth; /* union is for compatibility backward. * page_size_kb member variable should be removed in the future */ union { unsigned page_size_kb; unsigned page_size_bytes; }; ... }; How to repeat: Check storage/ndb/include/mgmapi/*.h. Suggested fix: Move definitions+files under src/mgmapi/ for clean include/mgmapi/*.h files.