[
https://issues.apache.org/jira/browse/HBASE-6659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444440#comment-13444440
]
stack commented on HBASE-6659:
------------------------------
Sorry. Meant to review this patch before it went in.
I have a bit of problem w/ this patch in that it loads up new payloads on top
of a channel we've been trying to shut down, the regionserver to master
heartbeat.
Here are some other comments:
Are all the splitLogFile changes to add an extra null arg needed? Could do an
overload for case where arg is non-null? (with default null)
Do we not have the methods getRegionsByServer in HBaseTestingUtility already --
or at least in the hbase cluster object?
Could TestHLogFiltering tests not be added to the TestHLog? Seems related?
We have to add this new regionserver call getLastFlushedSequenceId even though
the regionservers are heartbeating every second (by default)?
This seems like an ugly entanglement:
+ // For checking the latest flushed sequence id
+ protected final RegionServerStatusProtocol master;
... a master reference down deep inside in HLogSplitter. Its needed?
We make a call to the master per region from down inside HLogSplitter to ask
for last known sequence id?
What is difference between:
public FlushRegionResponse flushRegion(final RpcController controller,
final FlushRegionRequest request) throws ServiceException {
and
+ public void flushRegion(byte[] regionName)
+ throws NotServingRegionException, IOException {
Can you not do what you want w/ the first method? Its pb'd? Not sure how you
get at the second (You can pass null to the first IIRC... at least the second
should call the first.
Do we have to add
+ void updateLastFlushedSequenceIds(ServerName sn, ServerLoad hsl);
to MasterServices? I'd like it if we could keep MasterServices curt.
Everytime we add a new method, it makes for a bunch of changes in tests such as
the bleow and it also makes mocking that little bit tougher.
@Override
+ public void updateLastFlushedSequenceIds(ServerName sn, ServerLoad hsl) {
+ // no-op
+ }
Do we have to add a new map like this:
+ private final SortedMap<byte[], Long> flushedSequenceIdByRegion =
+ new ConcurrentSkipListMap<byte[], Long>(Bytes.BYTES_COMPARATOR);
Isn't there elsewhere where we keep list of regions out on cluster. Could we
add the sequenceid as an attribute on that Map rather than introduce a new one?
Putting sequenceid into serverload doesn't seem right. ServerLoad already is a
dumping ground. Now we have pbs, could add as attribute easy enough.
> Port HBASE-6508 Filter out edits at log split time
> --------------------------------------------------
>
> Key: HBASE-6659
> URL: https://issues.apache.org/jira/browse/HBASE-6659
> Project: HBase
> Issue Type: Bug
> Reporter: Zhihong Ted Yu
> Assignee: Zhihong Ted Yu
> Fix For: 0.96.0
>
> Attachments: 6508-v2.txt, 6508-v3.txt, 6508-v4.txt, 6508-v5.txt
>
>
> HBASE-6508 is for 0.89-fb branch.
> This JIRA ports the feature to trunk.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
