Last loc update by ivakegg · Pull Request #4 · ivakegg/accumulo

ivakegg · 2019-12-09T13:55:50Z

No description provided.

ivakegg · 2019-12-09T13:58:37Z

core/src/main/java/org/apache/accumulo/core/conf/Property.java

      "The number of threads on each tablet server available to retrieve"
          + " summary data, that is not currently in cache, from RFiles."),
+  TSERV_LASTLOCATION_UPDATE_TIME("tserver.lastlocation.update.time", "1000000", PropertyType.COUNT,
+          "The time in between tservers update last lcoation of tablets."),


lcoation -> location

ivakegg · 2019-12-09T14:07:27Z

server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java

+                                        String address, ZooLock zooLock, TServerInstance lastLocation) {
+
+    TabletMutator tablet = context.getAmple().mutateTablet(extent);
+    if(dfv.getNumEntries() > 0) {


Why are we passing this in and checking its number of entries?

That is what previous functions used and was just copied over. I wasn't sure about the intentions but I agree that it is not needed.

ivakegg · 2019-12-09T14:09:09Z

server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java

+    @Override
+    public void run() {
+      //this was more for testing
+      sleepUninterruptibly(10,


ivakegg · 2019-12-09T14:10:42Z

server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java

+        try {
+          //a pause in the action. this was taken from major compactor above
+          sleepUninterruptibly(getConfiguration().getTimeInMillis(Property.TSERV_LASTLOCATION_UPDATE_DELAY),
+                  TimeUnit.MILLISECONDS);


I don't like the idea of sleep uninterruptibly here. Does this not imply that the tserver cannot be stopped until this timer is complete?

This was done in major compactor for "Time a tablet server will sleep between checking which tablets need compaction". Just a small delay so it does not check right away. It might not be necessary.

ivakegg · 2019-12-09T14:14:16Z

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java


  }

+  public void updateLastLocation(){


need method comments for updateLastLocation and needsLastUpdate

ivakegg · 2019-12-09T14:16:07Z

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java

  }

+  public void updateLastLocation(){
+    synchronized (timeLock) {


why synchronize on timeLock. That is used for controlling access to persistedTime.

That was leftover past iterations of my testing. I suppose if I update perssitedTime inside updateLastLocation as you mentioned below it might be needed?

ivakegg · 2019-12-09T14:44:43Z

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java

+    //will keep looking for a better way to check if a tablet is old and needs to be updated or not. In the mean time, this seems to work and the property can be adjusted.
+    if (tabletTime.getAndUpdateTime() - t.tabletTime.getMetadataTime(persistedTime).getTime() >= tabletServer.getConfiguration().getCount(Property.TSERV_LASTLOCATION_UPDATE_TIME)) {
+      //updat persisted time, maybe not needed
+      persistedTime = tabletTime.getAndUpdateTime();


I would think we would do this when we do the update, not when we are checking whether we are to update.

Only did it here since I pass in the updated persistedTime to updateLastLocation. I suppose it could be moved and simplify the check function.

ivakegg · 2019-12-09T14:52:37Z

server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java

+    TabletMutator tablet = context.getAmple().mutateTablet(extent);
+    if(dfv.getNumEntries() > 0) {
+      //these two lines might not be needed for updating the last location
+      tablet.putFile(path, dfv);


What files are we changing? We are only changing the last location for the tablet, not the files. The time thing however may be needed.

I figured it wasn't needed but I did not want to break anything.

ivakegg · 2019-12-09T14:53:18Z

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java

+    synchronized (timeLock) {
+      Tablet t = tabletServer.getOnlineTablet(extent);
+      SortedMap<FileRef, DataFileValue> sm = t.getDatafiles();
+      for (Map.Entry<FileRef, DataFileValue> m : sm.entrySet()) {


A for loop over the files does not seem right here. We only need to change the last location of the tablet once.

This will be removed since I will not pass in dfv or fileref anymore.

ivakegg · 2019-12-09T16:19:36Z

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java


+  public void updateLastLocation(){
+    //update persisted time, maybe not needed
+    //persistedTime = tabletTime.getAndUpdateTime();


I think you need this update now such that the persisted time is current.

ivakegg · 2019-12-09T16:21:16Z

server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java

+
+            Tablet tablet = entry.getValue();
+            //unsure if synchronized block is needed or not
+            synchronized (tablet) {


since we are updating the time stuff, I think this is required.

Add last_loc_update Add comments made requested changes made further revisions made further revisions changes for testing changes for testing final changes Add method comments

…_loc_update

…st_loc_test_update2

ivakegg commented Dec 9, 2019

View reviewed changes

Add last_loc_update

3122c2d

Add last_loc_update Add comments made requested changes made further revisions made further revisions changes for testing changes for testing final changes Add method comments

Manno15 force-pushed the last_loc_update branch from b75868a to 3122c2d Compare December 10, 2019 17:56

Jeffrey Manno added 25 commits December 19, 2019 12:42

Small time change

5ed469e

Refactored and make some changes

f083774

testing update

922f061

revert and add last location to MetaDataStore

6358910

remove leftover function

bf7ef29

delete that pesky line

e217ce8

Merge branch 'master' of https://github.com/apache/accumulo into last…

52e069f

…_loc_update

remove update last location on compaction

2343bc5

Formatting change

a8c1bf3

testing clear last

36cdff0

small change

95c062e

small change

7137065

Merge branch 'last_loc_update' of github.com:Manno15/accumulo into la…

a40e096

…st_loc_test_update2

small change

d42ef0a

change

e5a7222

revert to original change

e90a371

clear previous last location before assigning new one

700ed61

refactor State Stores

dc45cc1

remove commmented code

1f08b35

add commments

afc5ee2

remove logs

ba87f3d

remove old setLocations function

a81e560

Refactor state store and update Last Location changes

849cf27

remove LastLoc Functions

c679160

remove putLoc Functions

d031a01

Jeffrey Manno and others added 8 commits March 23, 2020 09:41

remove collection<assigments>

3951339

remove collection<assigments> in setFuture

28b8d96

revert previous changes on setFutureLoc

edc19bd

Add suspending tablets to ample and in stateStore Function

f9f7e74

remove functions not used and finalize changes for refactors

f8b2beb

remove collection<assignments> from setFuture

66d93f7

Deleted more unused code. Various requested changes

71240b5

Merge branch 'master' into last_loc_update

98defb3

Manno15 deleted the last_loc_update branch May 29, 2020 12:20

Conversation

ivakegg commented Dec 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants