citusdata / citusdata/citus

Fix & improve vanilla tests (parallel_schedule) to benefit more from postgres's tests

Open
#4,086 2 comments 3 reactions 0 assignees View on GitHub
vanilla test
Dominant language
C
Stars
12.8k
Forks
794
Avg merge
2d 14h
Merged PRs (30d)
31

Description

Lastly, when I'm running vanilla tests to test the executor behavior around citus local tables development branch, I hit to some bugs, reproduced them for reference tables on master as can be seen in below issues:
* https://github.com/citusdata/citus/issues/4085 (fixed)
* https://github.com/citusdata/citus/issues/4089 (fixed)
* https://github.com/citusdata/citus/issues/4090 (fixed)
* https://github.com/citusdata/citus/issues/4150 (fixed)
* https://github.com/citusdata/citus/issues/4084
* https://github.com/citusdata/citus/issues/4091
* https://github.com/citusdata/citus/issues/4092
* https://github.com/citusdata/citus/issues/4093

Normally, when executing vanilla tests, we don't even have citus installed. I simply made some changes that @halilozanakgul suggested in `pg_regress.c` and simply changed the behavior in `utility_hook.c` to convert every created table to a citus local table as below:

```diff
@@ -519,6 +522,17 @@ multi_ProcessUtility(PlannedStmt *pstmt,
}
PG_END_TRY();

+ /* AllCitusLocalTable could be managed by hidden guc */
+ if (IsA(parsetree, CreateStmt) && AllCitusLocalTable)
+ {
+ /*
+ * We might execute sub CREATE TABLE commands in future ?
+ * Lets prevent infinite recursion.
+ */
+ AllCitusLocalTable = false;
+
+ CreateStmt *createTableStmt = (CreateStmt *) parsetree;
+ Oid relationId = RangeVarGetRelid(createTableStmt->relation, NoLock, false);
+ CreateCitusLocalTable(relationId);
+
+ AllCitusLocalTable = true;
+ }
```

We could introduce 3 (+1 when citus local tables are merged) release test items to test the behavior in single node citus (i.e only the coordinator is added):
1) Just create citus, don't intercept `CREATE TABLE` commands, test the native postgres behavior on pure postgres tables when citus is installed (@halilozanakgul found some bugs already by following this approach)
2) In post process of utility hook, convert every table to a reference table automatically
3) In post process of utility hook, convert every table to a distributed table automatically

But then, we know that we would have a lot of regression test diffs due to lots of sql commands/queries that we don't support currently. But at least, we can do:

1) Fix `pg_regress_multi.pl` to create the citus automatically when running vanilla tests without doing any manual changes in `pg_regress.c`
2)
* Remove vanilla tests from ci schedule and execute vanilla test scenarios described above at the end of each release, collect & investigate core dumps' outputs.
* Or don't remove vanilla tests from ci schedule, but don't check for regression diffs, only check if a crash is happened, and print core dumps

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.