ChinmaySKulkarni commented on a change in pull request #923:
URL: https://github.com/apache/phoenix/pull/923#discussion_r505879419
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -4006,6 +4037,33 @@ private PhoenixConnection
upgradeSystemTask(PhoenixConnection metaConnection)
"ALTER TABLE " + taskTableFullName + " SET " + TTL +
"=" + TASK_TABLE_TTL);
clearCache();
}
+ // If SYSTEM.TASK does not have disabled regions split policy,
+ // set it up here while upgrading it
+ try (Admin admin = metaConnection.getQueryServices().getAdmin()) {
Review comment:
Also, it would be better to change the DDL statement
(CQSI.CREATE_TASK_METADATA) to reflect the new SplitPolicy:
HTableDescriptor.SPLIT_POLICY + "='" + SystemTaskSplitPolicy.class.getName()
+ "',\n"
and introduce a new class `SystemTaskSplitPolicy` which is equivalent to
`DisabledRegionSplitPolicy` for now, but can be easily changed in the future if
required without unloading and reloading the coproc.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -4006,6 +4037,33 @@ private PhoenixConnection
upgradeSystemTask(PhoenixConnection metaConnection)
"ALTER TABLE " + taskTableFullName + " SET " + TTL +
"=" + TASK_TABLE_TTL);
clearCache();
}
+ // If SYSTEM.TASK does not have disabled regions split policy,
+ // set it up here while upgrading it
+ try (Admin admin = metaConnection.getQueryServices().getAdmin()) {
Review comment:
So new clusters would always come up with the correct split policy for
SYSTEM.TASK and <=4.15 clusters would go through this path
(upgradeSystemTask()) and loads the correct split policy. That way, we don't
need the change inside ensureTableCreated() and it is much cleaner.
One corner case (highly unlikely, but still possible) is if a cluster
already has SYSTEM.TASK split. In that case, we might want to throw an
exception or maybe merge regions of SYSTEM.TASK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
