[CARBONDATA-1912] Handling lock issues for alter rename operation #1734
Conversation
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2378/ |
|
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1162/ |
|
retest this please |
|
retest sdv please |
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2382/ |
|
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1166/ |
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2389/ |
|
retest this please |
|
retest sdv please |
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2403/ |
| try { | ||
| if (carbonTable != null) { | ||
| AlterTableUtil | ||
| .revertRenameTableChanges(oldTableIdentifier, |
There was a problem hiding this comment.
please follow code convention:
AlterTableUtil.revertRenameTableChanges(
xxx,
yyy
)
| renameBadRecords(newTableName, oldTableName, oldDatabaseName) | ||
| } | ||
| } catch { | ||
| case e: Exception => |
There was a problem hiding this comment.
use finally, without catch
|
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2589/ |
|
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2590/ |
ManoharVanam
force-pushed
the
defect1
branch
from
15f1d4b
to
ea2fddf
Compare
|
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2616/ |
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2428/ |
|
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1207/ |
|
@ManoharVanam...what is need of releasing the locks in catch block when finally block assures that lock is released? |
|
@manishgupta88 If operation is success, unlock will try to delete lock file from old table ( which doesn't exists) so error will be printed on spark sql console. So release old table lock only in case of exception. |
|
@ManoharVanam ...In that case can we change the logger error to warning in case of non existence of file and check whether still the warning log is getting printed. If warning logs are not getting printed in the console then we can make change in the logic instead changing the logic |
| if (carbonTable != null) { | ||
| AlterTableUtil | ||
| .revertRenameTableChanges(oldTableIdentifier, | ||
| try { |
There was a problem hiding this comment.
Still it is unknown, when exception is raised before rename or after rename.
So better handle unlocking at common place and logic and change table path if rename is success.
ManoharVanam
force-pushed
the
defect1
branch
2 times, most recently
from
98fd3a6
to
c1eafd5
Compare
|
LGTM....will merge after all builds run |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2549/ |
|
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2709/ |
|
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2710/ |
|
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2711/ |
|
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1324/ |
|
LGTM |
Modified code to release all old table acquired locks in case of any exception. So release all old table locks only in case of any exception instead of unlocking in finally block This closes apache#1734
Description : In case of any exception in alter rename operation, need to release all acquired locks
Solution : Need to release all old table acquired locks in case of any exception. So release all old table locks only in case of any exception instead of unlocking in finally block
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.