Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PHOENIX-6963 : Bootstrap last ddl timestamp for indexes #1612

Merged
merged 5 commits into from
May 31, 2023
Merged

PHOENIX-6963 : Bootstrap last ddl timestamp for indexes #1612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b39e8d
bootstrap last ddl timestamp for indexes
May 30, 2023
432d35d
review fixes
May 31, 2023
ab67381
review fixes 2
May 31, 2023
5047642
use EnvironmentEdgeManager
May 31, 2023
de4b9b4
checkstyle fixes
May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,12 @@ private void verifyExpectedCellValue(byte[] rowKey, byte[] syscatBytes,

@Test
public void testLastDDLTimestampBootstrap() throws Exception {
Long testStartTime = EnvironmentEdgeManager.currentTimeMillis();
//Create a table, view, and index
String schemaName = "S_" + generateUniqueName();
String tableName = "T_" + generateUniqueName();
String viewName = "V_" + generateUniqueName();
String indexName = "I_" + generateUniqueName();
String fullTableName = SchemaUtil.getTableName(schemaName, tableName);
String fullViewName = SchemaUtil.getTableName(schemaName, viewName);
try (Connection conn = getConnection(false, null)) {
Expand All @@ -697,21 +699,37 @@ public void testLastDDLTimestampBootstrap() throws Exception {
conn.createStatement().execute(
"CREATE VIEW " + fullViewName + " AS SELECT * FROM " + fullTableName);

conn.createStatement().execute("CREATE INDEX " + indexName + " ON " + fullTableName + " (KV1) ASYNC");

//Now we null out any existing last ddl timestamps
nullDDLTimestamps(conn);

//now get the row timestamps for each header row
long tableTS = getRowTimestampForMetadata(conn, schemaName, tableName,
PTableType.TABLE);
long viewTS = getRowTimestampForMetadata(conn, schemaName, viewName, PTableType.VIEW);
long indexTS = getRowTimestampForMetadata(conn, schemaName, indexName, PTableType.INDEX);
palashc marked this conversation as resolved.
Show resolved Hide resolved
assertTrue(tableTS > testStartTime);
assertTrue(viewTS > testStartTime);
assertTrue(indexTS > testStartTime);

UpgradeUtil.bootstrapLastDDLTimestamp(conn.unwrap(PhoenixConnection.class));
// bootstrap last ddl timestamp for tables and views
UpgradeUtil.bootstrapLastDDLTimestampForTablesAndViews(conn.unwrap(PhoenixConnection.class));
long actualTableTS = getLastTimestampForMetadata(conn, schemaName, tableName,
PTableType.TABLE);
long actualViewTS = getLastTimestampForMetadata(conn, schemaName, viewName,
PTableType.VIEW);
long actualIndexTS = getLastTimestampForMetadata(conn, schemaName, indexName,
PTableType.INDEX);
assertEquals(tableTS, actualTableTS);
assertEquals(viewTS, actualViewTS);
// only tables and views were bootstrapped
assertEquals(0L, actualIndexTS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should actualIndexTS be NULL or 0? I think it should be null.

Copy link
Contributor Author

@palashc palashc May 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is returned using a rs.getLong() in getLastTimestampForMetadata call which returns 0 if the actual value was sql null.


// bootstrap last ddl timestamp for indexes
UpgradeUtil.bootstrapLastDDLTimestampForIndexes(conn.unwrap(PhoenixConnection.class));
actualIndexTS = getLastTimestampForMetadata(conn, schemaName, indexName, PTableType.INDEX);
assertEquals(indexTS, actualIndexTS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am missing something.
indexTS is the timestamp after we set LAST_DDL_TIMESTAMP to null via nullDDLTimestamps method. So it should be NULL.
actualIndexTS is the timestamp after we call UpgradeUtil.bootstrapLastDDLTimestampForIndexes. So it should the LAST_ROW_TIMESTAMP() for this index.
So how are they supposed to be equal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shahrs87 indexTS is the PHOENIX_ROW_TIMESTAMP() of the header row of the index in syscat (it is the timestamp of the empty column) and indicates the last time the row was modified. After we call UpgradeUtil.bootstrapLastDDLTimestampForIndexes, this same value for the row is used to populate the LAST_DDL_TIMESTAMP column which is captured in actualIndexTS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad, I thought indexTS is calculated from getLastTimestampForMetadata method.


}
}
Expand Down
3 changes: 2 additions & 1 deletion phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4061,7 +4061,7 @@ protected PhoenixConnection upgradeSystemCatalogIfRequired(PhoenixConnection met
PhoenixDatabaseMetaData.SYSTEM_CATALOG, MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0,
PhoenixDatabaseMetaData.CHANGE_DETECTION_ENABLED
+ " " + PBoolean.INSTANCE.getSqlTypeName());
UpgradeUtil.bootstrapLastDDLTimestamp(metaConnection);
UpgradeUtil.bootstrapLastDDLTimestampForTablesAndViews(metaConnection);

boolean isNamespaceMapping =
SchemaUtil.isNamespaceMappingEnabled(null, getConfiguration());
Expand Down Expand Up @@ -4122,6 +4122,7 @@ protected PhoenixConnection upgradeSystemCatalogIfRequired(PhoenixConnection met
metaConnection = addColumnsIfNotExists(metaConnection, PhoenixDatabaseMetaData.SYSTEM_CATALOG,
MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0,
PhoenixDatabaseMetaData.STREAMING_TOPIC_NAME + " " + PVarchar.INSTANCE.getSqlTypeName());
UpgradeUtil.bootstrapLastDDLTimestampForIndexes(metaConnection);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how this UpgradeUtil code works and when it is executed? If it is executed after the upgrade then the server version would be equal to MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0. But adding DDL timestamps to table/view happened in 4.16.0 version and there also we have similar condition currentServerSideTableTimeStamp < MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0 so this code clearly works.

}
return metaConnection;
}
Expand Down
46 changes: 35 additions & 11 deletions phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@
import java.util.Set;
import java.util.concurrent.TimeoutException;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -2857,23 +2859,45 @@ public static void updateViewIndexIdColumnDataTypeFromShortToLong(
}
}

//When upgrading to Phoenix 4.16 or 5.1, make each existing table's DDL timestamp equal to its
// last updated row timestamp.
public static void bootstrapLastDDLTimestamp(Connection metaConnection) throws SQLException {
//When upgrading to Phoenix 4.16 or 5.1, make each existing table's/view's DDL timestamp equal
//to its last updated row timestamp.
public static void bootstrapLastDDLTimestampForTablesAndViews(Connection metaConnection)
throws SQLException {
bootstrapLastDDLTimestamp(metaConnection,
new String[]{PTableType.TABLE.getSerializedValue(),
PTableType.VIEW.getSerializedValue()});
}

//When upgrading to Phoenix 5.2, make each existing index's DDL timestamp equal to its
//last updated row timestamp.
public static void bootstrapLastDDLTimestampForIndexes(Connection metaConnection)
throws SQLException {
bootstrapLastDDLTimestamp(metaConnection,
new String[]{PTableType.INDEX.getSerializedValue()});
}

/**
* Sets the LAST_DDL_TIMESTAMP for the metadata header rows for all objects of the given table
* types to their PHOENIX_ROW_TIMESTAMP()
* @param metaConnection a {@link PhoenixConnection}
* @param tableTypes array of serialized {@link PTableType} values
*/
private static void bootstrapLastDDLTimestamp(Connection metaConnection, String[] tableTypes)
throws SQLException {
String tableTypesString = Stream.of(tableTypes).collect(
Collectors.joining("','", "'", "'")).toString();
String pkCols = TENANT_ID + ", " + TABLE_SCHEM +
", " + TABLE_NAME + ", " + COLUMN_NAME + ", " + COLUMN_FAMILY;
", " + TABLE_NAME + ", " + COLUMN_NAME + ", " + COLUMN_FAMILY;
final String upsertSql =
"UPSERT INTO " + SYSTEM_CATALOG_NAME + " (" + pkCols + ", " +
LAST_DDL_TIMESTAMP + ")" + " " +
"SELECT " + pkCols + ", PHOENIX_ROW_TIMESTAMP() FROM " + SYSTEM_CATALOG_NAME + " " +
"WHERE " + TABLE_TYPE + " " + " in " + "('" + PTableType.TABLE.getSerializedValue()
+ "', '" + PTableType.VIEW.getSerializedValue() + "')";
LOGGER.info("Setting DDL timestamps for tables and views to row timestamps");
"UPSERT INTO " + SYSTEM_CATALOG_NAME + " (" + pkCols + ", " + LAST_DDL_TIMESTAMP + ")"
+ " " + "SELECT " + pkCols + ", PHOENIX_ROW_TIMESTAMP() FROM " + SYSTEM_CATALOG_NAME
+ " " + "WHERE " + TABLE_TYPE + " " + " in " + "(" + tableTypesString + ")";
LOGGER.info("Setting DDL timestamps for table_type={} to row timestamps", tableTypesString);
try (PreparedStatement stmt = metaConnection.prepareStatement(upsertSql)) {
stmt.execute();
metaConnection.commit();
}
LOGGER.info("Setting DDL timestamps for tables and views is complete");
LOGGER.info("Setting DDL timestamps for table_type={} is complete", tableTypesString);
}

public static boolean tableHasKeepDeleted(PhoenixConnection conn, String pTableName)
Expand Down