Connect Orca via a planner hook

[whitehawk]

Connect Orca via a planner hook

Problem:
Orca's code was strongly coupled with the standard_planner() code. It introduced
difficulties for moving Orca's functionality into a shared lib.

This patch:

1. moves the invocation of Orca planner out from the standard_planner() into a
   hook in gp_orca shared lib;
2. moves the init procedure of Orca to the first call of the hook (thus the init
   is done when the backend has already been initialized including its memory
   protection means);
3. moves the deinit procedure of Orca into a callback, that is registered at the
   init (thus the deinit is done right before ShutdownPostgres() is called, where
   it was before).
4. adds clang-format config file

[bandetto]

Shouldn't we bring src/backend/gporca/.clang-format from original ORCA and use it in this extension as well?

[whitehawk]

Shouldn't we bring src/backend/gporca/.clang-format from original ORCA and use it in this extension as well?

It looks that current clang-format settings are targeted to cpp code mostly. On C code it may work not well, for example it spoils the C-style comments, like below:

	/*
   * When optimizer_use_gpdb_allocators is on, at least 2MB of above will be
   * tracked by vmem tracker later, so do not recount them.
   */

So for C code I'd rather use pgindent. Besides, it is not the 'core' Orca code.

[bandetto]

Seems like CI tests are skipped here, but locally, I can't find/create ORCA extension to CREATE and use it. Since there is no SQL file (like build/share/postgresql/extension/gporca--0.1.sql) installed, all queries are planned by planner. Is this task to address this issue?

[RekGRpth]

Seems like CI tests are skipped here, but locally, I can't find/create ORCA extension to CREATE and use it. Since there is no SQL file (like build/share/postgresql/extension/gporca--0.1.sql) installed, all queries are planned by planner. Is this task to address this issue?

To use ORCA, it is enough to place it in shared libraries, no create extension was expected!

[bandetto]

To use ORCA, it is enough to place it in shared libraries, no create extension was expected!

postgres=# explain (costs off) select * from t1;
                QUERY PLAN                
------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Seq Scan on t1
 Optimizer: Postgres-based planner
(3 rows)

postgres=# show optimizer;
 optimizer 
-----------
 on
(1 row)

[whitehawk]

To use ORCA, it is enough to place it in shared libraries, no create extension was expected!

postgres=# explain (costs off) select * from t1;
                QUERY PLAN                
------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Seq Scan on t1
 Optimizer: Postgres-based planner
(3 rows)

postgres=# show optimizer;
 optimizer 
-----------
 on
(1 row)

Hm, I'm getting correct results:

postgres=# create table t1(a int);
NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
CREATE TABLE
postgres=# explain (costs off) select * from t1;
                QUERY PLAN                
------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Seq Scan on t1
 Optimizer: GPORCA
(3 rows)

postgres=# show optimizer;
 optimizer 
-----------
 on
(1 row)

postgres=# 

what steps do you use?

[bandetto]

what steps do you use

Looks like the install-gp-orca target didn't properly add ORCA to preloaded libraries. Adding it manually fixed the issue.

Maybe use something more robust than just sed? For example, a quick hack to get the previous GUC value and append ORCA to it:

SHARED_LIBS=$(shell gpconfig -s shared_preload_libraries | sed -n '3s/.*Coordinator value: \(.*\)/\1/p')

install-gp-orca:
	...
	gpconfig -c shared_preload_libraries -m '$(SHARED_LIBS),gp_orca'

[RekGRpth]

sed

puts in shared library in default config, any You should create new cluster to use it, while

gpconfig

puts in shared library in existing cluster

[bandetto]

You should create new cluster to use it, while

I see. But I find this method fragile nonetheless. Maybe we should break it down?

For example, check if there is already a cluster running, and if there is a uncommented shared_preload_libraries in the config somewhere, append ORCA to it, otherwise insert shared_preload_libraries line in the config, along with modifying the sample configuration?

[whitehawk]

You should create new cluster to use it, while

I see. But I find this method fragile nonetheless. Maybe we should break it down?

For example, check if there is already a cluster running, and if there is a uncommented shared_preload_libraries in the config somewhere, append ORCA to it, otherwise insert shared_preload_libraries line in the config, along with modifying the sample configuration?

It sounds like out of the scope of this ticket. If needed.

[bandetto]

It sounds like out of the scope of this ticket. If needed.

Yeah, it does. I thought it should been noted. Maybe it would be good to have a README for these things.

[RekGRpth]

adds clang-format config file

I think formatting is a separate task, moreover .clang-format file from this patch is not included in

gpdb/src/tools/fmt

Line 67 in 72059d2

_find_result=$(find src/backend/gporca src/backend/gpopt src/include/gpopt -name .clang-format)

[whitehawk]

adds clang-format config file

I think formatting is a separate task, moreover .clang-format file from this patch is not included in

gpdb/src/tools/fmt

Line 67 in 72059d2

_find_result=$(find src/backend/gporca src/backend/gpopt src/include/gpopt -name .clang-format)

fmt will be fixed in scope of https://tracker.yandex.ru/ADBDEV-6627. I think we can add the file now, as the question was rised by one of the reviewers. Anyway, we will squash the delta when finally committing the feature.