Skip to content

[bug](regression) fix regression test test-table-version in cloud_p0 #33462

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

Merged
xy720 merged 4 commits into from
Apr 11, 2024
Merged

[bug](regression) fix regression test test-table-version in cloud_p0 #33462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f74ff0e
save
xy720 Apr 10, 2024
a43f28e
save
xy720 Apr 10, 2024
b2585e6
save
xy720 Apr 10, 2024
b39c183
save
xy720 Apr 10, 2024
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
3 changes: 2 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,12 +2300,13 @@ public boolean checkPartitionNameExist(String partitionName, boolean isTempParti

// drop temp partition. if needDropTablet is true, tablets of this temp partition
// will be dropped from tablet inverted index.
public void dropTempPartition(String partitionName, boolean needDropTablet) {
public Partition dropTempPartition(String partitionName, boolean needDropTablet) {
Partition partition = getPartition(partitionName, true);
if (partition != null) {
partitionInfo.dropPartition(partition.getId());
tempPartitions.dropPartition(partitionName, needDropTablet);
}
return partition;
}

/*
Expand Down
22 changes: 16 additions & 6 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1795,10 +1795,11 @@ public void dropPartition(Database db, OlapTable olapTable, DropPartitionClause

// drop
long recycleTime = 0;

Partition partition = null;
if (isTempPartition) {
olapTable.dropTempPartition(partitionName, true);
partition = olapTable.dropTempPartition(partitionName, true);
} else {
Partition partition = null;
if (!clause.isForceDrop()) {
partition = olapTable.getPartition(partitionName);
if (partition != null) {
Expand All @@ -1812,14 +1813,23 @@ public void dropPartition(Database db, OlapTable olapTable, DropPartitionClause
}
}
}
olapTable.dropPartition(db.getId(), partitionName, clause.isForceDrop());
partition = olapTable.dropPartition(db.getId(), partitionName, clause.isForceDrop());
if (!clause.isForceDrop() && partition != null) {
recycleTime = Env.getCurrentRecycleBin().getRecycleTimeById(partition.getId());
}
}
long version = olapTable.getNextVersion();
long versionTime = System.currentTimeMillis();
olapTable.updateVisibleVersionAndTime(version, versionTime);

long version = olapTable.getVisibleVersion();
Copy link
Contributor

@gavinchou gavinchou Apr 10, 2024

Choose a reason for hiding this comment

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

Is this thread-safe?

Copy link
Member Author

@xy720 xy720 Apr 10, 2024

Choose a reason for hiding this comment

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

Users must obtain write lock of OlapTable before calling the dropPartition method.

Copy link
Member Author

@xy720 xy720 Apr 10, 2024

Choose a reason for hiding this comment

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

I found that the commit transaction holds a commit lock for the table. May be I need to hold a commit lock here?

long versionTime = olapTable.getVisibleVersionTime();
// Only update table version if drop a non-empty partition
if (partition != null && partition.hasData()) {
versionTime = System.currentTimeMillis();
if (Config.isNotCloudMode()) {
version = olapTable.getNextVersion();
olapTable.updateVisibleVersionAndTime(version, versionTime);
}
}

// log
DropPartitionInfo info = new DropPartitionInfo(db.getId(), olapTable.getId(), partitionName, isTempPartition,
clause.isForceDrop(), recycleTime, version, versionTime);
Expand Down
21 changes: 18 additions & 3 deletions regression-test/suites/table_p0/test_table_version.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ suite("test_table_version") {
PARTITION BY RANGE(`date`)
(PARTITION p201701_1000 VALUES [('0000-01-01'), ('2017-02-01')),
PARTITION p201702_2000 VALUES [('2017-02-01'), ('2017-03-01')),
PARTITION p201703_all VALUES [('2017-03-01'), ('2017-04-01')))
PARTITION p201703_3000 VALUES [('2017-03-01'), ('2017-04-01')),
PARTITION p201704_all VALUES [('2017-04-01'), ('2017-05-01')))
DISTRIBUTED BY HASH(`user_id`) BUCKETS 2
PROPERTIES ('replication_num' = '1') ;
"""
Expand All @@ -49,19 +50,33 @@ suite("test_table_version") {
assertEquals(2, visibleVersion);

sql """
alter table ${tableNameNum} drop partition p201703_all force;
insert into ${tableNameNum} values(1,"2017-03-15",1);
"""
visibleVersion = getTableVersion(dbId,tableNameNum);
assertEquals(3, visibleVersion);

// drop an empty partition will not add table version
sql """
alter table ${tableNameNum} drop partition p201704_all force;
"""
visibleVersion = getTableVersion(dbId,tableNameNum);
assertEquals(3, visibleVersion);

// drop an non-empty partition will add table version
sql """
alter table ${tableNameNum} drop partition p201703_3000 force;
"""
visibleVersion = getTableVersion(dbId,tableNameNum);
assertEquals(4, visibleVersion);

sql """
ALTER TABLE ${tableNameNum} ADD TEMPORARY PARTITION p201702_2000_1 VALUES [('2017-02-01'), ('2017-03-01'));
"""
sql """
ALTER TABLE ${tableNameNum} REPLACE PARTITION (p201702_2000) WITH TEMPORARY PARTITION (p201702_2000_1);
"""
visibleVersion = getTableVersion(dbId,tableNameNum);
assertEquals(4, visibleVersion);
assertEquals(5, visibleVersion);

sql """drop table if exists `${tableNameNum}`"""
}