-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
[Enhancement] Support use table_name to filter scan in information_schema.tables #45351
Conversation
Signed-off-by: HangyuanLiu <460660596@qq.com>
# Conflicts: # be/src/exec/schema_scanner/schema_tables_scanner.cpp
if (table.isNativeTableOrMaterializedView()) { | ||
tableLocker.unLockTablesWithIntensiveDbLock(db, Lists.newArrayList(((OlapTable) table).getId()), | ||
LockType.READ); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Potential deadlock due to nested locking mechanisms without a consistent lock acquisition order.
You can modify the code like this:
// Before introducing new lock (tableLocker) inside the loop for tables,
// ensure there's a global policy or mechanism to avoid deadlocks.
// One approach could be ensuring that locks are always acquired in a consistent order and possibly reducing the scope where locks are held.
// For example, rather than acquiring a second lock inside the loop while still holding the first lock,
// consider restructuring the code to release the database lock before working on individual tables if feasible.
// This suggestion points towards a conceptual modification since the exact fix would need a detailed understanding of the overall locking strategy and application structure.
// Potential approach:
for (String dbName : result.authorizedDbs) {
Database db = metadataMgr.getDb(catalogName, dbName);
if (db == null) {
continue;
}
List<String> tableNames = metadataMgr.listTableNames(catalogName, dbName);
// Release the first lock before acquiring more specific table locks
// Assuming some mechanism or rearrangement enables us to do so safely
for (String tableName : tableNames) {
if (request.isSetTable_name() && !tableName.equals(request.getTable_name())) {
continue;
}
Locker tableLocker = new Locker();
try {
BasicTable table = metadataMgr.getBasicTable(catalogName, dbName, tableName);
if (table == null) {
continue;
}
tableLocker.lockTablesWithIntensiveDbLock(db, Lists.newArrayList(((OlapTable) table).getId()), LockType.READ);
// process table...
} catch (Exception e) {
LOG.warn(e.getMessage());
} finally {
tableLocker.unLockTablesWithIntensiveDbLock(db, Lists.newArrayList(((OlapTable) table).getId()), LockType.READ);
}
}
}
// Note: This does not represent a complete solution but highlights the importance of addressing potential deadlock scenarios by carefully managing lock acquisition and release sequences.
Quality Gate passedIssues Measures |
[FE Incremental Coverage Report]✅ pass : 44 / 48 (91.67%) file detail
|
[BE Incremental Coverage Report]✅ pass : 2 / 2 (100.00%) file detail
|
fe/fe-core/src/main/java/com/starrocks/service/InformationSchemaDataSource.java
Show resolved
Hide resolved
@Mergifyio backport branch-3.3 |
@Mergifyio backport branch-3.2 |
@Mergifyio backport branch-3.1 |
@Mergifyio backport branch-3.0 |
@Mergifyio backport branch-2.5 |
✅ Backports have been created
|
✅ Backports have been created
|
✅ Backports have been created
|
…hema.tables (StarRocks#45351) Signed-off-by: HangyuanLiu <460660596@qq.com>
Why I'm doing:
What I'm doing:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: