PG-799: Implement smgr GUC variable and chaining #23
Conversation
| // setup a dummy chain with md, for tools | ||
| storage_manager_chain.chain[0] = MdSMgrId; | ||
| storage_manager_chain.size = 1; |
There was a problem hiding this comment.
Tools might need to setup a chain as well. Is this something future improvements?
There was a problem hiding this comment.
Yes, tools are ignored for now. We'll have to revisit that question later, and probably for WAL too - we need some kind of extension support for tools.
| @@ -175,6 +174,18 @@ smgrshutdown(int code, Datum arg) | |||
| } | |||
| } | |||
|
|
|||
| #define SMGR_CHAIN_LOOKUP(SMGR_METHOD) while(chain_index < reln->smgr_chain.size && smgrsw[reln->smgr_chain.chain[chain_index]].SMGR_METHOD == NULL) chain_index++ | |||
| #define SMGR_CHAIN_CHECK_END_ASSERT() Assert(chain_index < reln->smgr_chain.size) | |||
There was a problem hiding this comment.
Should we have a check somewhere that the method we ended up isn't NULL? In case of all the chain has NULL for create, for example. As far as I can see there is a special case for shutdown but the rest of the methods should be implemented at least at the end of the chain
There was a problem hiding this comment.
Yes, I'll add a check for "tail" smgrs.
src/backend/storage/smgr/smgr.c
Outdated
| #define SMGR_CHAIN_CHECK_END_ASSERT() Assert(chain_index < reln->smgr_chain.size) | ||
|
|
||
| void | ||
| smgr_open_internal(SMgrRelation reln, SmgrChainIndex chain_index) |
There was a problem hiding this comment.
It bothers me a bit that functions to be called from external entities have _internal in the name. Maybe smgr_open_next?
Plus it'd be nice to reference chain_index implicitly w/o passing it as an arg. But I can't come up with how) Maybe at least have some macros so user function don't have to write chain_index + 1 all the time?
There was a problem hiding this comment.
"Plus it'd be nice to reference chain_index implicitly w/o passing it as an arg." - we can use macros, but that won't be pretty, I tried.
Or we can go with global state, but I wanted to avoid that, because if a storage manager ends up calling sql functions, like we did when we stored metadata in the catalog, we'll end up with multiple levels of smgr calls, and a global state would break - unless I also add handling for this, which would again complicate things.
No description provided.