You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "srinireddy2020 (via GitHub)" <gi...@apache.org> on 2026/03/24 17:39:59 UTC
[PR] HBASE-30016 Cached table regions locations are not using in TableSnapshotInputFormatImpl#getSplits [hbase]
srinireddy2020 opened a new pull request, #7975:
URL: https://github.com/apache/hbase/pull/7975
In org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormatImpl#getSplits(org.apache.hadoop.hbase.client.Scan, org.apache.hadoop.hbase.snapshot.SnapshotManifest, java.util.List<org.apache.hadoop.hbase.client.RegionInfo>, org.apache.hadoop.fs.Path, org.apache.hadoop.conf.Configuration, org.apache.hadoop.hbase.util.RegionSplitter.SplitAlgorithm, int)
Connection obj is closing early. RegionLocator references are using after connection obj closed.
so that cached table regions locations are not using.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30016 Cached table regions locations are not using in TableSnapshotInputFormatImpl#getSplits [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #7975:
URL: https://github.com/apache/hbase/pull/7975#discussion_r2988900383
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
+ if (localityEnabled && useRegionLoc) {
+ Configuration newConf = new Configuration(conf);
+ newConf.setInt("hbase.hconnection.threads.max", 1);
+
connection = ConnectionFactory.createConnection(newConf);
regionLocator = connection.getRegionLocator(htd.getTableName());
/* Get all locations for the table and cache it */
regionLocator.getAllRegionLocations();
- } finally {
- if (connection != null) {
- connection.close();
- }
}
- }
- List<InputSplit> splits = new ArrayList<>();
- for (RegionInfo hri : regionManifests) {
- // load region descriptor
- List<String> hosts = null;
- if (localityEnabled) {
- if (regionLocator != null) {
- /* Get Location from the local cache */
- HRegionLocation location = regionLocator.getRegionLocation(hri.getStartKey(), false);
-
- hosts = new ArrayList<>(1);
- hosts.add(location.getHostname());
- } else {
- hosts = calculateLocationsForInputSplit(conf, htd, hri, tableDir);
+ for (RegionInfo hri : regionManifests) {
+ // load region descriptor
+ List<String> hosts = null;
+ if (localityEnabled) {
+ if (regionLocator != null) {
+ /* Get Location from the local cache */
+ HRegionLocation location = regionLocator.getRegionLocation(hri.getStartKey(), false);
+
+ hosts = new ArrayList<>(1);
+ hosts.add(location.getHostname());
+ } else {
+ hosts = calculateLocationsForInputSplit(conf, htd, hri, tableDir);
+ }
}
- }
- if (numSplits > 1) {
- byte[][] sp = sa.split(hri.getStartKey(), hri.getEndKey(), numSplits, true);
- for (int i = 0; i < sp.length - 1; i++) {
+ if (numSplits > 1) {
+ byte[][] sp = sa.split(hri.getStartKey(), hri.getEndKey(), numSplits, true);
+ for (int i = 0; i < sp.length - 1; i++) {
+ if (
+ PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), sp[i],
+ sp[i + 1])
+ ) {
+
+ Scan boundedScan = new Scan(scan);
+ if (scan.getStartRow().length == 0) {
+ boundedScan.withStartRow(sp[i]);
+ } else {
+ boundedScan.withStartRow(
+ Bytes.compareTo(scan.getStartRow(), sp[i]) > 0 ? scan.getStartRow() : sp[i]);
+ }
+
+ if (scan.getStopRow().length == 0) {
+ boundedScan.withStopRow(sp[i + 1]);
+ } else {
+ boundedScan.withStopRow(Bytes.compareTo(scan.getStopRow(), sp[i + 1]) < 0
+ ? scan.getStopRow()
+ : sp[i + 1]);
+ }
+
+ splits.add(new InputSplit(htd, hri, hosts, boundedScan, restoreDir));
+ }
+ }
+ } else {
if (
- PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), sp[i], sp[i + 1])
+ PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(),
+ hri.getStartKey(), hri.getEndKey())
) {
- Scan boundedScan = new Scan(scan);
- if (scan.getStartRow().length == 0) {
- boundedScan.withStartRow(sp[i]);
- } else {
- boundedScan.withStartRow(
- Bytes.compareTo(scan.getStartRow(), sp[i]) > 0 ? scan.getStartRow() : sp[i]);
- }
-
- if (scan.getStopRow().length == 0) {
- boundedScan.withStopRow(sp[i + 1]);
- } else {
- boundedScan.withStopRow(
- Bytes.compareTo(scan.getStopRow(), sp[i + 1]) < 0 ? scan.getStopRow() : sp[i + 1]);
- }
-
- splits.add(new InputSplit(htd, hri, hosts, boundedScan, restoreDir));
+ splits.add(new InputSplit(htd, hri, hosts, scan, restoreDir));
}
}
- } else {
- if (
- PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), hri.getStartKey(),
- hri.getEndKey())
- ) {
-
- splits.add(new InputSplit(htd, hri, hosts, scan, restoreDir));
- }
+ }
+ } finally {
+ if (connection != null) {
Review Comment:
i believe all changes are just because we added an extra block {} and code got shifted?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30016 Cached table regions locations are not using in TableSnapshotInputFormatImpl#getSplits [hbase]
Posted by "srinireddy2020 (via GitHub)" <gi...@apache.org>.
srinireddy2020 commented on code in PR #7975:
URL: https://github.com/apache/hbase/pull/7975#discussion_r2995354776
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
+ if (localityEnabled && useRegionLoc) {
+ Configuration newConf = new Configuration(conf);
+ newConf.setInt("hbase.hconnection.threads.max", 1);
+
connection = ConnectionFactory.createConnection(newConf);
regionLocator = connection.getRegionLocator(htd.getTableName());
/* Get all locations for the table and cache it */
regionLocator.getAllRegionLocations();
- } finally {
- if (connection != null) {
- connection.close();
- }
}
- }
- List<InputSplit> splits = new ArrayList<>();
- for (RegionInfo hri : regionManifests) {
- // load region descriptor
- List<String> hosts = null;
- if (localityEnabled) {
- if (regionLocator != null) {
- /* Get Location from the local cache */
- HRegionLocation location = regionLocator.getRegionLocation(hri.getStartKey(), false);
-
- hosts = new ArrayList<>(1);
- hosts.add(location.getHostname());
- } else {
- hosts = calculateLocationsForInputSplit(conf, htd, hri, tableDir);
+ for (RegionInfo hri : regionManifests) {
+ // load region descriptor
+ List<String> hosts = null;
+ if (localityEnabled) {
+ if (regionLocator != null) {
+ /* Get Location from the local cache */
+ HRegionLocation location = regionLocator.getRegionLocation(hri.getStartKey(), false);
+
+ hosts = new ArrayList<>(1);
+ hosts.add(location.getHostname());
+ } else {
+ hosts = calculateLocationsForInputSplit(conf, htd, hri, tableDir);
+ }
}
- }
- if (numSplits > 1) {
- byte[][] sp = sa.split(hri.getStartKey(), hri.getEndKey(), numSplits, true);
- for (int i = 0; i < sp.length - 1; i++) {
+ if (numSplits > 1) {
+ byte[][] sp = sa.split(hri.getStartKey(), hri.getEndKey(), numSplits, true);
+ for (int i = 0; i < sp.length - 1; i++) {
+ if (
+ PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), sp[i],
+ sp[i + 1])
+ ) {
+
+ Scan boundedScan = new Scan(scan);
+ if (scan.getStartRow().length == 0) {
+ boundedScan.withStartRow(sp[i]);
+ } else {
+ boundedScan.withStartRow(
+ Bytes.compareTo(scan.getStartRow(), sp[i]) > 0 ? scan.getStartRow() : sp[i]);
+ }
+
+ if (scan.getStopRow().length == 0) {
+ boundedScan.withStopRow(sp[i + 1]);
+ } else {
+ boundedScan.withStopRow(Bytes.compareTo(scan.getStopRow(), sp[i + 1]) < 0
+ ? scan.getStopRow()
+ : sp[i + 1]);
+ }
+
+ splits.add(new InputSplit(htd, hri, hosts, boundedScan, restoreDir));
+ }
+ }
+ } else {
if (
- PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), sp[i], sp[i + 1])
+ PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(),
+ hri.getStartKey(), hri.getEndKey())
) {
- Scan boundedScan = new Scan(scan);
- if (scan.getStartRow().length == 0) {
- boundedScan.withStartRow(sp[i]);
- } else {
- boundedScan.withStartRow(
- Bytes.compareTo(scan.getStartRow(), sp[i]) > 0 ? scan.getStartRow() : sp[i]);
- }
-
- if (scan.getStopRow().length == 0) {
- boundedScan.withStopRow(sp[i + 1]);
- } else {
- boundedScan.withStopRow(
- Bytes.compareTo(scan.getStopRow(), sp[i + 1]) < 0 ? scan.getStopRow() : sp[i + 1]);
- }
-
- splits.add(new InputSplit(htd, hri, hosts, boundedScan, restoreDir));
+ splits.add(new InputSplit(htd, hri, hosts, scan, restoreDir));
}
}
- } else {
- if (
- PrivateCellUtil.overlappingKeys(scan.getStartRow(), scan.getStopRow(), hri.getStartKey(),
- hri.getEndKey())
- ) {
-
- splits.add(new InputSplit(htd, hri, hosts, scan, restoreDir));
- }
+ }
+ } finally {
+ if (connection != null) {
Review Comment:
yes
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30016 Cached table regions locations are not using in TableSnapshotInputFormatImpl#getSplits [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #7975:
URL: https://github.com/apache/hbase/pull/7975#discussion_r2988883454
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
+ if (localityEnabled && useRegionLoc) {
+ Configuration newConf = new Configuration(conf);
+ newConf.setInt("hbase.hconnection.threads.max", 1);
+
Review Comment:
can we do try-with-resources here
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
Review Comment:
can we do try-with-resources here
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
+ if (localityEnabled && useRegionLoc) {
+ Configuration newConf = new Configuration(conf);
+ newConf.setInt("hbase.hconnection.threads.max", 1);
+
Review Comment:
can we do try-with-resources here
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30016 Cached table regions locations are not using in TableSnapshotInputFormatImpl#getSplits [hbase]
Posted by "srinireddy2020 (via GitHub)" <gi...@apache.org>.
srinireddy2020 commented on code in PR #7975:
URL: https://github.com/apache/hbase/pull/7975#discussion_r2995435568
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSnapshotInputFormatImpl.java:
##########
@@ -424,74 +424,76 @@ public static List<InputSplit> getSplits(Scan scan, SnapshotManifest manifest,
Connection connection = null;
RegionLocator regionLocator = null;
- if (localityEnabled && useRegionLoc) {
- Configuration newConf = new Configuration(conf);
- newConf.setInt("hbase.hconnection.threads.max", 1);
- try {
+ List<InputSplit> splits = new ArrayList<>();
+ try {
Review Comment:
Based on condition only resources are initializing.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org