You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2024/08/22 01:53:13 UTC
[PR] Stops waiting on scans during tablet close. [accumulo]
keith-turner opened a new pull request, #4819:
URL: https://github.com/apache/accumulo/pull/4819
Before this change when a tablet was closing it would prevent new scans from starting and wait for running scans to finish. This could cause the tablet to become unavailable for long periods of time. After this change tablets will no longer wait on running scans to complete. They will set something to interrupt the thread and then disable the client scan session which prevents the client form ever seeing any data after the tablet is closed. Once all scan sessions are disabled the tablet will proceed to close. Disabling the scan session will cause the client side scanner to eventually switch to the new tablet server where the tablet is loaded.
Its possible that threads may be left running on the tablet server therefore it will be important to also implement #4756
fixes #4757
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1733437507
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
+ List<ScanDataSource> runningScans = new ArrayList<>(this.activeScans);
+
+ runningScans.removeIf(scanDataSource -> {
+ boolean disabled = scanDataSource.disableClientSession();
Review Comment:
Updated variable and method names in b6dd442 to better match the behavior of the code. Also removed a level indirection (a `BooleanSupplier`) I thought I needed to avoid passing around the SessionManager, however found it was not needed. Removing this level indirection makes the code easier to follow along with the better names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1729422670
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
Review Comment:
> I wonder if we want to capture the time that the interrupt flag was set to true, and wait some period of time before forcing the tablet to close
I was initially going to wait but decided against it for the following reasons.
1. Once the interrupt flag is set its a matter of luck if the scan threads gets additional data or not. So waiting may not result in clients getting additional data.
2. Since the tablet no longer allows new scans to start, we do not know how many of those are currently being held up.
3. Attempting to close the clients scan session will naturally introduce some delay because of the way reservations work on scan sessions. Will wait for any client currently holding a reservation to relinquish it, which clients do every second by default but this is configurable. After a client does relinquish the reservation on the session it can never be reserved again with the new disable code added in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#issuecomment-2307020292
I'm wondering if we should continue to wait for scans to complete for the root and metadata tables before unloading them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1733031141
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
+ List<ScanDataSource> runningScans = new ArrayList<>(this.activeScans);
+
+ runningScans.removeIf(scanDataSource -> {
+ boolean disabled = scanDataSource.disableClientSession();
Review Comment:
Isn't it possible that the scan session is disabled (reservations disallowed) when returning false too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1728921052
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
Review Comment:
The call to `ScanDataSource.interrupt()` on line 1039 above sets the interrupt flag on the iterators to true. The issue we are addressing here is zombie scans, where the interrupt flag is being set, but not checked in some user supplied iterator (#4757). I wonder if we want to capture the time that the interrupt flag was set to true, and wait some period of time before forcing the tablet to close giving scans that are behaving correctly a chance to succeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #4819:
URL: https://github.com/apache/accumulo/pull/4819
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1729422670
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
Review Comment:
> I wonder if we want to capture the time that the interrupt flag was set to true, and wait some period of time before forcing the tablet to close
I was initially going to wait but decided against it for the following reasons.
1. Once the interrupt flag is set its a matter of luck if the scan threads gets additional data or not. So waiting may not result in clients getting additional data.
2. Since the tablet no longer allows new scans to start, we do not know how many of those are currently being held up.
3. Attempting to disable the clients scan session will naturally introduce some delay because of the way reservations work on scan sessions. Will wait for any client currently holding a reservation to relinquish it, which clients do every second by default but this is configurable. After a client does relinquish the reservation on the session it can never be reserved again with the new disable code added in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Stops waiting on scans during tablet close. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4819:
URL: https://github.com/apache/accumulo/pull/4819#discussion_r1733377231
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1038,13 +1039,31 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScan.interrupt();
}
+ // create a copy so that it can be whittled down as client sessions are disabled
+ List<ScanDataSource> runningScans = new ArrayList<>(this.activeScans);
+
+ runningScans.removeIf(scanDataSource -> {
+ boolean disabled = scanDataSource.disableClientSession();
Review Comment:
Yeah that is possible. I am going to take a pass at renaming things. It really initiating disabling the session, going to see if I can name it to reflect that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org