[TRAFODION-2873] Removed usage of system new from LobGlobals and other classes #1405
Conversation
… derived from a heap. Moved xLObHdfsRequest also to be derived from a heap
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2372/ |
core/sql/exp/ExpLOBaccess.h
Outdated
| @@ -589,7 +589,7 @@ class ExLobGlobals | |||
| { | |||
| public : | |||
|
|
|||
| ExLobGlobals(); | |||
| ExLobGlobals(NAHeap *lobHeap=NULL); | |||
There was a problem hiding this comment.
It is better to avoid the default value for this parameter.
There was a problem hiding this comment.
Yes this was only done to be able to compile the call in ExpLobProcess.cpp. Currently this code is unused and is kept around in case we need to offload any processing to another process. This code, invokes LobGlobals and there is no heap available. So I had to make it default just for this unused code. WIll put a comment that it shoudl always pass a heap. In anycase a caller cannot initialize the LobGlobals without passing in a caller's heap - i.e the only way to initialize is by calling ExpLOBinterfaceInit or ExpLOBoper::initLobGlobal. Both require a heap to get passed in.
There was a problem hiding this comment.
To @selvaganesang's point, perhaps we should leave this without a default, and then code NULL explicitly in ExpLobProcess.cpp?
There was a problem hiding this comment.
Sure that's another way. Will do that.
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2372/ |
core/sql/cli/Cli.cpp
Outdated
| Int32 rc= ExpLOBoper::initLOBglobal(exLobGlob,currContext.exHeap(),&currContext,hdfsServer,hdfsPort); | ||
| if (rc) | ||
| { | ||
| { |
There was a problem hiding this comment.
It doesn't hurt anything, but I'm curious: Why two sets of braces? Looks like one would do?
core/sql/cli/Cli.cpp
Outdated
| Int32 rc= ExpLOBoper::initLOBglobal(exLobGlob,currContext.exHeap(),&currContext,hdfsServer,hdfsPort); | ||
| if (rc) | ||
| { | ||
| { |
core/sql/cli/Cli.cpp
Outdated
| Int32 rc= ExpLOBoper::initLOBglobal(exLobGlob,currContext.exHeap(),&currContext,hdfsServer,hdfsPort); | ||
| if (rc) | ||
| { | ||
| { |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2373/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2373/ |
| @@ -575,7 +575,7 @@ Lng32 main(Lng32 argc, char *argv[]) | |||
| // setup log4cxx | |||
| QRLogger::initLog4cxx(QRLogger::QRL_LOB); | |||
| // initialize lob globals | |||
| lobGlobals = new ExLobGlobals(); | |||
| lobGlobals = new ExLobGlobals(NULL); | |||
There was a problem hiding this comment.
Thanks for avoiding the default value for the param. But, I think sending heap as NULL might make this program to fail. Have you tested this program? If you have tested this program already, then this PR is ready to be merged.
There was a problem hiding this comment.
The ExpLOBprocess.cpp is dormant code for now. If we enable it and create a LOB process to offload some operations like GC etc, we will need to create and setup globals , send messages form the master process to it etc . For now I just had to ensure it compiled.
ExpOBInterfaceInit will take a parent heap and derive a lobHeap from it. ExLobGlobals will get created from this heap and store this heap pointer for use by other classes.
ExpLOBinterfaceCleanup destroys the LOB globals and will utilize the heap pointer stored within it.
In cli/Cli.cpp , Removed the initialization/destruction of LOB glabals from the ::createLOB interface and moved it out before the interface is called to be consistent with other LOB API usage.