Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,11 @@ public void testTabletInsertRedirectsToInternalLeaderEndpoint() throws Exception
session.insert(createTablet(2L));

final Session underlyingSession = InternalSessionITUtils.getUnderlyingSession(session);
final Map<?, TEndPoint> tableModelDeviceIdToEndpoint =
final Map<?, Map<?, TEndPoint>> tableModelDeviceIdToEndpointByDatabase =
InternalSessionITUtils.getSessionField(underlyingSession, "tableModelDeviceIdToEndpoint");
assertEquals(1, tableModelDeviceIdToEndpointByDatabase.size());
final Map<?, TEndPoint> tableModelDeviceIdToEndpoint =
tableModelDeviceIdToEndpointByDatabase.values().iterator().next();
assertEquals(1, tableModelDeviceIdToEndpoint.size());
assertTrue(leaderEndPoints.contains(tableModelDeviceIdToEndpoint.values().iterator().next()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,63 @@ public static void main(String[] args)
}
}

@Test
public void testInsertTabletIntoSpecifiedDatabase()
throws IoTDBConnectionException, StatementExecutionException {
try (ITableSession session = EnvFactory.getEnv().getTableSessionConnection()) {
session.executeNonQueryStatement(
"CREATE TABLE db1.tablet_database (tag1 STRING TAG, m1 DOUBLE FIELD)");
session.executeNonQueryStatement(
"CREATE TABLE db2.tablet_database (tag1 STRING TAG, m1 DOUBLE FIELD)");

List<String> columnNames = Arrays.asList("tag1", "m1");
List<TSDataType> dataTypes = Arrays.asList(TSDataType.STRING, TSDataType.DOUBLE);
List<ColumnCategory> columnCategories =
Arrays.asList(ColumnCategory.TAG, ColumnCategory.FIELD);
Tablet tablet = new Tablet("tablet_database", columnNames, dataTypes, columnCategories, 2);
for (int row = 0; row < 2; row++) {
tablet.addTimestamp(row, row);
tablet.addValue("tag1", row, "tag:" + row);
tablet.addValue("m1", row, row * 1.0);
}

session.insert(tablet, "db1");
session.executeNonQueryStatement("USE db1");
session.insert(tablet, "db2");
Tablet currentDatabaseTablet =
new Tablet("tablet_database", columnNames, dataTypes, columnCategories, 1);
currentDatabaseTablet.addTimestamp(0, 2);
currentDatabaseTablet.addValue("tag1", 0, "tag:2");
currentDatabaseTablet.addValue("m1", 0, 2.0);
session.insert(currentDatabaseTablet, null);

try (SessionDataSet dataSet =
session.executeQueryStatement("SELECT * FROM db1.tablet_database")) {
assertTrue(dataSet.hasNext());
dataSet.next();
assertTrue(dataSet.hasNext());
dataSet.next();
assertTrue(dataSet.hasNext());
dataSet.next();
assertFalse(dataSet.hasNext());
}
try (SessionDataSet dataSet =
session.executeQueryStatement("SELECT * FROM db2.tablet_database")) {
assertTrue(dataSet.hasNext());
dataSet.next();
assertTrue(dataSet.hasNext());
dataSet.next();
assertFalse(dataSet.hasNext());
}
try (SessionDataSet dataSet = session.executeQueryStatement("SHOW CURRENT_DATABASE")) {
assertTrue(dataSet.hasNext());
assertEquals("db1", dataSet.next().getField(0).getStringValue());
}

assertThrows(StatementExecutionException.class, () -> session.insert(tablet, ""));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please assert the status code here, not just the exception type. "" currently returns 301, the generic failure code, so this assertion would still pass if the call failed for an unrelated reason.

Missing cases:

  • Tree-prefixed names. Try "root.xxx", "ROOT.xxx" and "Root.xxx" while both tree database root.xxx and table database xxx exist. Assert the error code, and assert that no device appears under root.xxx.** (see the comment on StatementGenerator).
  • The grouped path with an explicit database. No test runs insertRelationalTabletByGroup with a database set. The new unit test uses a single device. In the IT, each explicit-database insert starts with an empty route map for that database, so it always goes to the default connection. Extending SessionCacheLeaderTest with a multi-device tablet whose devices have different cached leaders, written to an explicit database that differs from the session's, would cover the grouping, the redirect bookkeeping and the per-group requests carrying the database.

}
}

private static void insertRelationalTabletPerformanceTest()
throws IoTDBConnectionException, StatementExecutionException {
try (ITableSession session = EnvFactory.getEnv().getTableSessionConnection()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,12 @@ public void testInternalPullConsumerPollsAcrossDataNodes() throws Exception {
session.executeNonQueryStatement("flush");

final Session underlyingSession = InternalSessionITUtils.getUnderlyingSession(session);
final Map<?, TEndPoint> deviceToEndpoint =
final Map<?, Map<?, TEndPoint>> deviceToEndpointByDatabase =
InternalSessionITUtils.getSessionField(
underlyingSession, "tableModelDeviceIdToEndpoint");
leaders.addAll(deviceToEndpoint.values());
deviceToEndpointByDatabase
.values()
.forEach(deviceToEndpoint -> leaders.addAll(deviceToEndpoint.values()));
}

Assert.assertTrue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,15 @@
boolean enableRPCCompression,
int connectionTimeoutInMs,
Map<String, TEndPoint> deviceIdToEndpoint,
Map<IDeviceID, TEndPoint> tabletModelDeviceIdToEndpoint,
Map<IDeviceID, TEndPoint> tableModelDeviceIdToEndpoint,
INodeSupplier nodeSupplier)
throws IoTDBConnectionException;

void openWithTableModelDatabaseCache(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This adds an internal cache-sharing hook to ISession, a public interface that users program against, and its nested-map parameter exposes the cache layout. The only callers are in SessionPool. The method has to be on the interface only because getSession() declares ISession session = queue.poll();. Opening the new connection through a local Session would avoid that:

Session newSession = constructNewSession();
newSession.openWithTableModelDatabaseCache(...);
session = newSession;

tryConstructNewSession() already uses a Session.

Separately, the new abstract methods here and the new ITableSession.insert(Tablet, String) break every third-party implementation of these interfaces at compile time. ISession already uses default methods for this kind of evolution (e.g. isEnableCacheLeader). insert(Tablet, String) could be a default method that delegates to insert(tablet) when database == null and throws UnsupportedOperationException otherwise.

boolean enableRPCCompression,

Check warning on line 78 in iotdb-client/isession/src/main/java/org/apache/iotdb/isession/ISession.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Abbreviation in name 'enableRPCCompression' must contain no more than '2' consecutive capital letters.

See more on https://sonarcloud.io/project/issues?id=apache_iotdb&issues=AaHOv4dKuDHHi52pnTma&open=AaHOv4dKuDHHi52pnTma&pullRequest=19237
int connectionTimeoutInMs,
Map<String, TEndPoint> deviceIdToEndpoint,
Map<String, Map<IDeviceID, TEndPoint>> tableModelDeviceIdToEndpoint,
INodeSupplier nodeSupplier)
throws IoTDBConnectionException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ public interface ITableSession extends AutoCloseable {
*/
void insert(Tablet tablet) throws StatementExecutionException, IoTDBConnectionException;

/**
* Inserts a {@link Tablet} into the specified database for this invocation only.
*
* <p>The database does not become the current database of this session and subsequent operations
* are unaffected.
*
* @param tablet the tablet containing time-series data to be inserted.
* @param database the target database for this insertion; if null, the current database of this
* session is used.
* @throws StatementExecutionException if an error occurs while executing the statement.
* @throws IoTDBConnectionException if there is an issue with the IoTDB connection.
*/
void insert(Tablet tablet, String database)
throws StatementExecutionException, IoTDBConnectionException;

/**
* Executes a non-query SQL statement, such as a DDL or DML command.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,13 @@
private static final double CONVERT_THRESHOLD = 0.5;
private static final double SAMPLE_PROPORTION = 0.05;
private static final int MIN_RECORDS_SIZE = 40;
private static final String UNSPECIFIED_DATABASE_KEY = "";

@SuppressWarnings("squid:S3077") // Non-primitive fields should not be "volatile"
protected volatile Map<String, TEndPoint> deviceIdToEndpoint;

@SuppressWarnings("squid:S3077") // Non-primitive fields should not be "volatile"
protected volatile Map<IDeviceID, TEndPoint> tableModelDeviceIdToEndpoint;
protected volatile Map<String, Map<IDeviceID, TEndPoint>> tableModelDeviceIdToEndpoint;

@SuppressWarnings("squid:S3077") // Non-primitive fields should not be "volatile"
protected volatile Map<TEndPoint, SessionConnection> endPointToSessionConnection;
Expand Down Expand Up @@ -666,6 +667,28 @@
Map<IDeviceID, TEndPoint> tableModelDeviceIdToEndpoint,
INodeSupplier nodesSupplier)
throws IoTDBConnectionException {
Map<String, Map<IDeviceID, TEndPoint>> databaseToDeviceIdToEndpoint = new ConcurrentHashMap<>();
if (tableModelDeviceIdToEndpoint != null) {
databaseToDeviceIdToEndpoint.put(
resolveTableModelRouteDatabase(null), tableModelDeviceIdToEndpoint);
}
openWithTableModelDatabaseCache(
enableThriftRpcCompaction,
connectionTimeoutInMs,
deviceIdToEndpoint,
databaseToDeviceIdToEndpoint,
nodesSupplier);
}

/** Opens this Session with a database-aware table-model route cache for internal callers. */
@Override
public synchronized void openWithTableModelDatabaseCache(
boolean enableThriftRpcCompaction,
int connectionTimeoutInMs,
Map<String, TEndPoint> deviceIdToEndpoint,
Map<String, Map<IDeviceID, TEndPoint>> tableModelDeviceIdToEndpoint,
INodeSupplier nodesSupplier)
throws IoTDBConnectionException {
if (!isClosed) {
return;
}
Expand Down Expand Up @@ -1420,12 +1443,20 @@
}
}

private SessionConnection getSessionConnection(IDeviceID deviceId)
private Map<IDeviceID, TEndPoint> getTableModelDatabaseRoutes(String routeDatabase) {
Map<IDeviceID, TEndPoint> databaseToEndpoint = tableModelDeviceIdToEndpoint.get(routeDatabase);
return databaseToEndpoint == null
? tableModelDeviceIdToEndpoint.computeIfAbsent(
routeDatabase, ignored -> new ConcurrentHashMap<>())
: databaseToEndpoint;
}

private SessionConnection getSessionConnection(
Map<IDeviceID, TEndPoint> databaseToEndpoint, IDeviceID deviceId)
throws IoTDBConnectionException {
TEndPoint endPoint;
if (enableRedirection
&& tableModelDeviceIdToEndpoint != null
&& (endPoint = tableModelDeviceIdToEndpoint.get(deviceId)) != null
&& (endPoint = databaseToEndpoint.get(deviceId)) != null
&& endPointToSessionConnection.containsKey(endPoint)) {
return endPointToSessionConnection.get(endPoint);
} else {
Expand Down Expand Up @@ -1465,12 +1496,13 @@
}
}
if (tableModelDeviceIdToEndpoint != null && !tableModelDeviceIdToEndpoint.isEmpty()) {
for (Iterator<Entry<IDeviceID, TEndPoint>> it =
tableModelDeviceIdToEndpoint.entrySet().iterator();
it.hasNext(); ) {
Entry<IDeviceID, TEndPoint> entry = it.next();
if (entry.getValue().equals(endPoint)) {
it.remove();
for (Map<IDeviceID, TEndPoint> deviceIdToEndpoint : tableModelDeviceIdToEndpoint.values()) {

Check warning on line 1499 in iotdb-client/session/src/main/java/org/apache/iotdb/session/Session.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Rename "deviceIdToEndpoint" which hides the field declared at line 178.

See more on https://sonarcloud.io/project/issues?id=apache_iotdb&issues=AaHOv2eKuDHHi52pnTmY&open=AaHOv2eKuDHHi52pnTmY&pullRequest=19237
for (Iterator<Entry<IDeviceID, TEndPoint>> it = deviceIdToEndpoint.entrySet().iterator();
it.hasNext(); ) {
Entry<IDeviceID, TEndPoint> entry = it.next();
if (Objects.equals(entry.getValue(), endPoint)) {
it.remove();
}
}
}
}
Expand Down Expand Up @@ -1503,15 +1535,15 @@
}
}

private void handleRedirection(IDeviceID deviceId, TEndPoint endpoint) {
private void handleRedirection(
Map<IDeviceID, TEndPoint> databaseToEndpoint, IDeviceID deviceId, TEndPoint endpoint) {
if (enableRedirection) {
// no need to redirection
if (UrlUtils.isWildcardAddress(endpoint.ip)) {
return;
}
if (!tableModelDeviceIdToEndpoint.containsKey(deviceId)
|| !tableModelDeviceIdToEndpoint.get(deviceId).equals(endpoint)) {
tableModelDeviceIdToEndpoint.put(deviceId, endpoint);
if (!endpoint.equals(databaseToEndpoint.get(deviceId))) {
databaseToEndpoint.put(deviceId, endpoint);
}
SessionConnection connection =
endPointToSessionConnection.computeIfAbsent(
Expand All @@ -1524,11 +1556,16 @@
}
});
if (connection == null) {
tableModelDeviceIdToEndpoint.remove(deviceId);
databaseToEndpoint.remove(deviceId, endpoint);
}
}
}

private String resolveTableModelRouteDatabase(String database) {
String routeDatabase = database == null ? this.database : database;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the route-cache key is the raw string, while the server lower-cases it, so "DB1" and "db1" get separate route maps for the same database. getTableModelDatabaseRoutes also calls computeIfAbsent before the server has validated the name. Every invalid name ("", "ROOT.x", typos) therefore leaves an empty entry in a map that a SessionPool shares across sessions. Lower-casing here, as the server does, and only creating the entry when a redirect is actually recorded would avoid both problems.

return routeDatabase == null ? UNSPECIFIED_DATABASE_KEY : routeDatabase;
}

private void handleQueryRedirection(TEndPoint endPoint) throws IoTDBConnectionException {
if (enableQueryRedirection) {
AtomicReference<IoTDBConnectionException> exceptionReference = new AtomicReference<>();
Expand Down Expand Up @@ -2907,11 +2944,24 @@
*/
public void insertRelationalTablet(Tablet tablet)
throws IoTDBConnectionException, StatementExecutionException {
insertRelationalTablet(tablet, null);
}

/**
* Inserts a relational {@link Tablet} into the specified database for this invocation only.
*
* @param tablet data batch
* @param database target database; if null, use the current database of this session
*/
public void insertRelationalTablet(Tablet tablet, String database)
throws IoTDBConnectionException, StatementExecutionException {

Check warning on line 2957 in iotdb-client/session/src/main/java/org/apache/iotdb/session/Session.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected @throws tag for 'IoTDBConnectionException'.

See more on https://sonarcloud.io/project/issues?id=apache_iotdb&issues=AaHN-38Fo2mj_z9fa0l6&open=AaHN-38Fo2mj_z9fa0l6&pullRequest=19237

Check warning on line 2957 in iotdb-client/session/src/main/java/org/apache/iotdb/session/Session.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected @throws tag for 'StatementExecutionException'.

See more on https://sonarcloud.io/project/issues?id=apache_iotdb&issues=AaHN-38Fo2mj_z9fa0l7&open=AaHN-38Fo2mj_z9fa0l7&pullRequest=19237
if (tablet.getRowSize() == 0) {
return;
}
// Capture the effective database before grouping so a later USE cannot change cached routes.
String routeDatabase = resolveTableModelRouteDatabase(database);
if (enableRedirection) {
insertRelationalTabletWithLeaderCache(tablet);
insertRelationalTabletWithLeaderCache(tablet, database, routeDatabase);
} else {
TSInsertTabletReq request = genTSInsertTabletReq(tablet, false, false);
if (request == null) {
Expand All @@ -2923,24 +2973,28 @@
return;
}
request.setWriteToTable(true);
setRelationalRequestDatabase(request, database);
try {
getDefaultSessionConnection().insertTablet(request);
} catch (RedirectException ignored) {
}
}
}

private void insertRelationalTabletWithLeaderCache(Tablet tablet)
private void insertRelationalTabletWithLeaderCache(
Tablet tablet, String database, String routeDatabase)
throws IoTDBConnectionException, StatementExecutionException {
Map<SessionConnection, Tablet> relationalTabletGroup = new HashMap<>();
if (tableModelDeviceIdToEndpoint.isEmpty()) {
Map<IDeviceID, TEndPoint> databaseToEndpoint = getTableModelDatabaseRoutes(routeDatabase);
if (databaseToEndpoint.isEmpty()) {
relationalTabletGroup.put(getDefaultSessionConnection(), tablet);
} else if (SessionUtils.isTabletContainsSingleDevice(tablet)) {
relationalTabletGroup.put(getSessionConnection(tablet.getDeviceID(0)), tablet);
relationalTabletGroup.put(
getSessionConnection(databaseToEndpoint, tablet.getDeviceID(0)), tablet);
} else {
for (int i = 0; i < tablet.getRowSize(); i++) {
IDeviceID iDeviceID = tablet.getDeviceID(i);
final SessionConnection connection = getSessionConnection(iDeviceID);
final SessionConnection connection = getSessionConnection(databaseToEndpoint, iDeviceID);
int finalI = i;
relationalTabletGroup.compute(
connection,
Expand Down Expand Up @@ -2975,13 +3029,16 @@
}
}
if (relationalTabletGroup.size() == 1) {
insertRelationalTabletOnce(relationalTabletGroup);
insertRelationalTabletOnce(relationalTabletGroup, database, databaseToEndpoint);
} else {
insertRelationalTabletByGroup(relationalTabletGroup);
insertRelationalTabletByGroup(relationalTabletGroup, database, databaseToEndpoint);
}
}

private void insertRelationalTabletOnce(Map<SessionConnection, Tablet> relationalTabletGroup)
private void insertRelationalTabletOnce(

Check failure on line 3038 in iotdb-client/session/src/main/java/org/apache/iotdb/session/Session.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=apache_iotdb&issues=AaHN-38Fo2mj_z9fa0l4&open=AaHN-38Fo2mj_z9fa0l4&pullRequest=19237
Map<SessionConnection, Tablet> relationalTabletGroup,
String database,
Map<IDeviceID, TEndPoint> databaseToEndpoint)
throws IoTDBConnectionException, StatementExecutionException {
Map.Entry<SessionConnection, Tablet> entry = relationalTabletGroup.entrySet().iterator().next();
SessionConnection connection = entry.getKey();
Expand All @@ -2996,6 +3053,7 @@
return;
}
request.setWriteToTable(true);
setRelationalRequestDatabase(request, database);
try {
connection.insertTablet(request);
} catch (RedirectException e) {
Expand All @@ -3006,7 +3064,8 @@
endPointMap.put(tablet.getDeviceID(i), endPointList.get(i));
}
}
endPointMap.forEach(this::handleRedirection);
endPointMap.forEach(
(deviceId, endpoint) -> handleRedirection(databaseToEndpoint, deviceId, endpoint));
} catch (IoTDBConnectionException e) {
if (endPointToSessionConnection != null && endPointToSessionConnection.size() > 1) {
// remove the broken session
Expand All @@ -3024,7 +3083,10 @@
@SuppressWarnings({
"squid:S3776"
}) // ignore Cognitive Complexity of methods should not be too high
private void insertRelationalTabletByGroup(Map<SessionConnection, Tablet> relationalTabletGroup)
private void insertRelationalTabletByGroup(
Map<SessionConnection, Tablet> relationalTabletGroup,
String database,
Map<IDeviceID, TEndPoint> databaseToEndpoint)
throws IoTDBConnectionException, StatementExecutionException {
List<CompletableFuture<Void>> completableFutures =
relationalTabletGroup.entrySet().stream()
Expand All @@ -3044,6 +3106,7 @@
return;
}
request.setWriteToTable(true);
setRelationalRequestDatabase(request, database);
InsertConsumer<TSInsertTabletReq> insertConsumer =
SessionConnection::insertTablet;
try {
Expand All @@ -3056,7 +3119,9 @@
endPointMap.put(subTablet.getDeviceID(i), endPointList.get(i));
}
}
endPointMap.forEach(this::handleRedirection);
endPointMap.forEach(
(deviceId, endpoint) ->
handleRedirection(databaseToEndpoint, deviceId, endpoint));
} catch (StatementExecutionException e) {
throw new CompletionException(e);
} catch (IoTDBConnectionException e) {
Expand Down Expand Up @@ -3101,6 +3166,12 @@
}
}

private static void setRelationalRequestDatabase(TSInsertTabletReq request, String database) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the null check is redundant. The generated setDatabase(null) leaves the field unset, because isSetDatabase() is database != null. More importantly, the database is attached at three separate call sites, each right after setWriteToTable(true), so a future send path can easily forget it. A single builder for relational tablet requests, taking the tablet and the database, would keep this in one place. That builder is also the natural home for the server-capability check suggested on client.thrift.

if (database != null) {
request.setDatabase(database);
}
}

/**
* insert the aligned timeseries data of a device. For each timestamp, the number of measurements
* is the same.
Expand Down
Loading
Loading